firefox-devtools / ux

Firefox DevTools UX Community
Mozilla Public License 2.0
103 stars 21 forks source link

Inline error annotations in Debugger #124

Closed digitarald closed 3 years ago

digitarald commented 4 years ago

https://phabricator.services.mozilla.com/D71337 is adding inline error annotations.

If an error is thrown the line gets annotated with an error icon that shows the error message + stack on hover.

The same mechanism could eventually also map console error and warnings to console locations to show them in context when loading a source and stepping through the code.

The idea would be to unblock landing the work soon and to file follow up bugs for UI polish.

@jasonLaster brought up a point, that the expression could be red underlined without an icon. Maybe the styling could be similar to matched strings, but in red?

image
digitarald commented 4 years ago

The latest design from the patch switched to curly underline: image

jasonLaster commented 4 years ago

Yep, I think microsoft word taught us that this is what errors look like and other products like vs code have reinforced it. I'd be fine removing the red line background as that might be a bit noisy for a feature that is just a bit of additional information.

violasong commented 4 years ago

Cool, looking good!

I see the curliness matches VSCode but it seems a bit more dramatic than it needs to be - I can experiment with a more minimal look.

The red background was making me think these were errors that would keep the app from running - if they're not that major then removing it makes sense.

Comparisons:

VSCode

image

Xcode

image

goog docs

image
jasonLaster commented 4 years ago

Thanks for the comparables.

These exceptions do stop the program at these points. Some examples of this are foo.doSomething where foo doesn't exist or doSomething is not real... In these cases the programs throw an exception and bail out.

stepanstava commented 4 years ago

I am just wondering if the underline without the line background doesn't get lost in more 'dense' sources? line-background

Btw. we could also discuss the styles of the tooltip? Now it copies styles from the console. What do you think? tooltip

Also, do you think when hovering over the underlined token the background should change? In pause state, there is this yellow background while hovering, not sure if it's needed to change background here. tokenHover

violasong commented 4 years ago

Ah, line background seems good to me. Looks like it might be a slightly different color than the Console error background at the moment - would be great to match it.

The tooltip with red styling looks a bit odd when juxtaposed with the other red row. I'd suggest the regular styling for that.

It would be good to give the token a hover background. We can use that same yellow for now, though it should probably be revisited separately (should probably be a blue instead).

stepanstava commented 4 years ago

Thanks Victoria!

I'll match the exception background color with the console error background color and keep the yellow background color on hover for now.

The tooltip with red styling looks a bit odd when juxtaposed with the other red row. I'd suggest the regular styling for that.

What did you mean by regular styling for the tooltip? Something like this? exc-tooltip Should the text color be changed? Now it's the same as the console text error color. Thank you.

violasong commented 4 years ago

This looks good! I had been thinking of making the tooltip black on white, but I think using the red text color helps relate it to the error row.

violasong commented 3 years ago

Looks like the associated bug is fixed? Closing unless we need a new polish bug.