[Beta UI] Identify segments that can't be marked as reviewed

[jftanner]

Describe what you are trying to accomplish and why in non technical terms
When using the new Review UI, some segments cannot be marked as reviewed because they haven't ended yet. When these are the only segments visible, the "Mark these items as reviewed" button appears to be broken.

Ongoing events are indicated by a spinning marker but this makes me thing "loading" rather than "ongoing", so I didn't realize what was happening until I looked in the code. It wasn't until I went back to check that I realized that the spinner meant that the event was still happening.

Describe the solution you'd like
There's three changes I would suggest:

• Using a different icon that conveys ongoing events without being confused for a loading spinner. (Not sure what this would be.)
• Updating button text to say "Mark finished events as reviewed"
• Hide the button when no unfinished events are displayed

Describe alternatives you've considered
Ideally, users should be able to mark ongoing events as reviewed, without needing to wait for them to be finished. For example: I know my wife is currently mowing the lawn, so I don't need to wait for her to finish before I mark that event.

However, I assume that the current filter is intentional. Perhaps because you don't want users to "review" things that haven't happened yet, or perhaps for a technical reason. Either way, the filter seems reasonable, so a clearer UI is all that's really necessary.

Additional context
It looks to me like the issue occurs in this filter:

      const itemsToMarkReviewed = currentItems
        ?.filter((seg) => seg.end_time)
        ?.map((seg) => seg.id);

Any segments not yet ended are filtered out of the itemsToMarkReviewed. Then, because the list is empty, there is no API call made. So even in the Network tab of browser devtools, the button doesn't look like it did anything besides reload the page.

[jftanner]

I'd be happy to contribute any/all of these fixes, if you approve of the proposed solution. Or I can try to implement a different fix, if you've got one that you prefer. :)

This project is incredibly valuable to me, so I'd love to pay it back any way I can.

[NickM-27]

The progress icon shows where the x minutes ago time usually shows. IMO this is pretty straightforward, given that the image is showing I don't know what would be loading. I agree if only in progress items are shown then no mark as reviewed button should be shown

and yes, you could see something you think is normal and mark it as reviewed but then that person ends up breaking in to your house or hitting your car or whatever the case may be

[jftanner]

IMO this is pretty straightforward

In hindsight it's obvious, yes. I certainly won't be confused about it again. But that spinner is very much hard-coded in my brain as "loading". I didn't know what it was loading, so I found it confusing, but I certainly didn't read it as "this event is still happening".

Is that a user training problem? Yeah, probably. But, IMO, the best designs are ones that make sense intuitively. So a different icon may help with that. (Or, alternatively, my brain is weird. That's the problem with having developers as testers: we don't think like most users.)

I agree if only in progress items are shown then no mark as reviewed button should be shown

And I see you've beaten me to the PR already. 😂

I know different projects have different preferences, so, for little stuff like this, which do you prefer:

• An issue, so you can decide how to handle it before any work starts?
• A PR with a plausible fix? (Mine would have probably changed the button text as well, but that's always changeable before merging.)

[NickM-27]

I think it depends. I would always suggest submitting an issue or discussion first if you don't want the time spent on what you think is a proper fix to be wasted in the event that the maintainers want to go a different route. If you don't care then a PR could be fine.

In general, with this UI we have spent half a year putting thought into it, working with a professional designer, and going through many iterations. If we spent time accepting or adjusting things in the UI that a single or few of users found confusing at first, think should be changed, etc. then we would end up making UI tweaks for an eternity. So when it comes to the UI specifically I would suggest a discussion first and not just a PR outright.

[jftanner]

Please don't take my comments (on this, or anything else like it) as "this is bad" or "you have to change this". It's just feedback on what I, personally, found confusing. If you never hear it again, just chalk it up to me being weird!

I think it depends [on how much time you're willing to waste]

Sounds good. That's what I tell folks on my repos at work, so it's easy for me to remember. 😁

we have spent half a year putting thought into [the UI], working with a professional designer

And it shows! It's really quite fantastic. I'm absolutely blown away by the quality in both design and functionality. It's very, very impressive work and you should all be proud.

I miss having an actual designer on my team; a good UX really needs one, but it so often gets left to developers/engineers. My design skills are mediocre at best, but I still have to do it. 😞

[...] things in the UI that a single or few of users found confusing at first [...]

Every time I conduct a user experience test, I'm baffled by the things users find confusing or unintuitive. Often, it's just one person being a little dense (as may well be the case here), but sometimes I have a whole bunch of users stumbling over something that I think is patently obvious. Not sure what that says about me. 😂

Which is a long way of saying that I 100% understand not wanting to take every single user comment as actionable feedback. Sometimes we users are dumb.