chore: make the milestone status a button #9255
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
| ? theme.palette.text.secondary | ||
| : theme.palette.text.primary, | ||
| '& svg': { | ||
| '&&&': { |
There was a problem hiding this comment.
I've seen this before: is this just a hack to override the specificity? In this case, why use a MUI button at all? Why not just use a regular button?
There was a problem hiding this comment.
Yep, it is. How do you feel about this instead? e60385c
There was a problem hiding this comment.
Nice! I think that's better, but why use pointer-events: none? Wouldn't it be better to use cursor: default? You're already using the disabled attribute, which makes it unreachable by tab anyway.
There was a problem hiding this comment.
This brings back our previous behavior of the button being "click-through" when disabled, which means that if the milestone is running or paused, and you click the disabled button, you still toggle the accordion. The cursor is still pointer because, again, we "click-through" the button and toggle the accordion.
Would you expect a different behavior?
There was a problem hiding this comment.
Ah, got it. Yeah, I can see the logic there. I'm not sure it's what I'd expect, but if it's UX approved, then I don't really have any arguments against it. maybe that it's unintuitive? Do disabled buttons still get hover styles? Because it might be strange if it looks like it can be interacted with but it can't? It can't get tab focus, so that's not an issue though.
There was a problem hiding this comment.
Do disabled buttons still get hover styles?
No, they don't. Those statuses act the same as before this PR.
I'm proceeding with this for now but we can revisit this later if needed.
There was a problem hiding this comment.
Great! My point was I think it'd be confusing if they did. So yeah, happy for you to merge it 😄
https://linear.app/unleash/issue/2-3251/make-the-milestone-status-action-link-and-icon-a-proper-button-that
Makes the milestone status a button while trying to keep most of the original design intact.