Move switch and diff buttons #139
Conversation
This reverts commit c1b8d2b.
…-switch-snapshot-and-diff-buttons
…-switch-snapshot-and-diff-buttons
…-switch-snapshot-and-diff-buttons
|
Not specifically PR feedback in need of change, but just some general design feedback. I find this orientation quite disorienting. I think I like the toggle on the bottom, but it feels like there is a lot of content on the top bar now. That was without changes. With changes, it shifts around a bit because of the line break. Though I am really glad that it linebreaks because we can see the entire branch name. I wonder if the snapshot toggle could live on the bottom of the screen, above the browser and mode selectors, and then keep the top line for controls. @MichaelArestad @ghengeveld |
There was a problem hiding this comment.
I noticed a bug. I don't know if it's specific to this or the base pr, but if you toggle the diff image off, it switches back after a short pause. I don't know for certain, but I imagine this could be caused by polling since it always happens after a short delay, multiple times even.
Update: confirmed, it happens any time the polling refreshes
Screen.Recording.2023-11-03.at.5.36.57.PM.mov
|
@weeksling Thanks for the thoroughness. I fixed the diff toggle bug in the upstream PR. As for the design, I'm working on some improvements in another ticket. The switch button is going to move to the right next to the diff toggle. The accept buttons are moving up above those diff controls. I'm truncating (with ellipsis) the branch name. That's on just a single line though. Should we allow wrapping to a second line @MichaelArestad? That would be a bit more complicated (it's not a native CSS feature) but seems like a good idea. |
No. Let's keep it to a single line for now for simplicity in both design and implementation. |
|
Closing in favor of #148 |
Telescoping on #138
📦 Published PR as canary version:
0.0.117--canary.139.93bc623.0✨ Test out this PR locally via:
npm install @chromaui/addon-visual-tests@0.0.117--canary.139.93bc623.0 # or yarn add @chromaui/addon-visual-tests@0.0.117--canary.139.93bc623.0