-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Beta UI] Identify segments that can't be marked as reviewed #11632
Comments
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. |
The progress icon shows where the 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 |
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.)
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:
|
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. |
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!
Sounds good. That's what I tell folks on my repos at work, so it's easy for me to remember. 😁
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. 😞
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. |
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:
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:
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.The text was updated successfully, but these errors were encountered: