★ wanayoo — archive 1999 https://github.com/firefox-devtools/debugger/issues/8049Nouvelle recherche | Portail wanayoo
Skip to content

/ debugger

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

Jump to bottom

[UX] WhyPaused component is displayed at an awkward place, and has low contrast #8049

Closed
fvsch opened this issue Mar 1, 2019 · 6 comments
Closed

[UX] WhyPaused component is displayed at an awkward place, and has low contrast #8049

fvsch opened this issue Mar 1, 2019 · 6 comments
Assignees
@derek-li
Labels
👋 polish 👋 quality discussion needs-ux

Comments

@fvsch
Copy link
Member

@fvsch fvsch commented Mar 1, 2019

whypaused_screenshot

Let's look at the contrast issue first, since it's more straightforward. On the light theme:

  • the main label has 3.10:1 contrast;
  • the error text has 1.91:1 contrast.

(We're aiming for a minimum of 4.5:1 contrast for both.)

Things would be better if we removed the opacity: 0.6 on that component.

Now, for the placement part. I would argue that:

  • Having this information in the Call Stack panel is a bit unexpected, in practice I wonder if it's not just ignored by most users?
  • The placement between the paged call stack and its "Expand rows" button is awkward, it creates a semantic break in the usual "limited content + 'show more' button" pattern.

It seems this component was moved here in #3111 to avoid another issue (vertical jump when that component is displayed or not). But I wonder if, with this change, we haven't lost a visible indicator of the "Paused on breakpoint|exception" state? In my experience nowadays the paused state can mostly be detected by looking at the Play/Pause icon, which is a weak signal. @jasonLaster, I think you've got similar feedback from other users?

So maybe we should revisit having a "Paused on..." information higher up, perhaps always visible even when the SecondaryPanes are closed (like the play/pause/skip buttons), and try to move WhyPaused there?
@claim claim bot added the not-available label Mar 1, 2019
@digitarald
Copy link
Contributor

@digitarald digitarald commented Mar 5, 2019

Came up in #8068 as well and I found this issue. Given how hard debugging already is to get into and be good at; we can do much better at clearly communicating state.

Requirements for a good warning

  1. High contrast, possibly building on the photon warnings style
  2. Primary element in the UI, above the fold and not in an accordion.
  3. Should not cause unnecessary visual noise when stepping (only when the why changes).
  4. Lean on existing paused state warnings from other debuggers which users are trained on

Existing work

Safari, also shows the function and file as reference and link:
image

Chrome, also shows meta data for the reason (which could be linked better):
image
image
image

VSCode shows paused state by having a conditional Debug toolbar on top.

image
@derek-li
Copy link
Contributor

@derek-li derek-li commented Mar 12, 2019

What's your guys opinion on moving the WhyPaused component back to it's original location before the #3111 change? @jasonLaster

I feel this way we could add the requirements for a more visible warning and even have the information available when the secondary pane is collapsed. It also conforms to the Chrome WhyPaused.
@digitarald
Copy link
Contributor

@digitarald digitarald commented Mar 12, 2019

That seems like the best approach, @derek-li. Apart from the warning styling, another consideration is to keep an eye on the visual noise while stepping, which #3111 was mainly about.
@derek-li
Copy link
Contributor

@derek-li derek-li commented Mar 13, 2019

/claim
@claim claim bot assigned derek-li Mar 13, 2019
@claim claim bot added the in progress label Mar 13, 2019
@claim
Copy link

@claim claim bot commented Mar 13, 2019

Thanks for claiming the issue! 👋

Here are some links for getting setup, contributing, and developing. We're always happy to answer questions in slack! If you become busy, feel free to /unclaim it.

🦊 Debugger team!
@fvsch
Copy link
Member Author

@fvsch fvsch commented Mar 13, 2019

At the very least it's worth experimenting with moving it back up, and trying to address the issues seen in #3111 in a different way. For example, while stepping the WhyPaused component should not disappear (even just briefly); that would mitigate most vertical jumpiness.
derek-li added a commit to derek-li/debugger.html that referenced this issue Mar 26, 2019
@derek-li
Fix for firefox-devtools#8049 (WIP)
5ab9228 
- Moved WhyPaused component to previous location (top of pane frames) for more visibility
- Reduced vertical jumpiness on re-renders/updates
- Minor changes to contrast
derek-li added a commit to derek-li/debugger.html that referenced this issue Mar 26, 2019
@derek-li
Merge branch 'master' into issue-firefox-devtools#8049
c3954e1
@derek-li derek-li mentioned this issue Mar 26, 2019
Merged
derek-li added a commit to derek-li/debugger.html that referenced this issue Apr 5, 2019
@derek-li
Merge branch 'master' into issue-firefox-devtools#8049
be78cf7
@claim claim bot removed the in progress label Apr 17, 2019
darkwing added a commit that referenced this issue Apr 17, 2019
@derek-li @darkwing
Issue #8049 Make WhyPaused more visible (#8163)
Loading status checks…
c2c176c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@derek-li derek-li

Labels
👋 polish 👋 quality discussion needs-ux
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Issue #8049 Make WhyPaused more visible
4 participants
@digitarald @darkwing @fvsch @derek-li