Skip to content
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

Align preference and notification styling with VSCode #10719

Merged
merged 1 commit into from
Feb 15, 2022

Conversation

msujew
Copy link
Member

@msujew msujew commented Feb 7, 2022

What it does

Updates our hovering and focus behavior for preferences and notifications to behave like the current version of VSCode.

How to test

Both of these tests should be performed for different light/dark themes. I changed the color slightly for preferences in the light theme compared to VSCode, since hovering and focus color were completely transparent. In my opinion this didn't really look good.


  1. Open the example app, hover over and click on one of the browser-endpoint notifications.
  2. Open the notification center, again, checking hover and click/focus behavior
  3. Assert that it aligns to VSCode's behavior

  1. Open the preference widget
  2. Hover over different preferences and click on them (focus). The More Actions... gear should only appear when a preference is focused.
  3. Change a preference so that it deviates from its original default value. Assert that the appearing vertical bar looks correct.
  4. When clicking on a referenced preference, the target preference should be displayed in the center of the widget and should automatically gain the focus.
  5. Assert that is aligns to VSCode's behavior

Review checklist

Reminder for reviewers

@msujew msujew added preferences issues related to preferences notifications issues related to notifications ui/ux issues related to user interface / user experience labels Feb 7, 2022
@msujew msujew force-pushed the msujew/align-notifications-preferences branch from df68883 to 98f6b51 Compare February 8, 2022 14:47
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the bug mentioned below, the changes look good. One other thing I noticed is that we currently allow selection of the text of the preference name and description where VSCode does not, but that doesn't have to be addressed here.

packages/preferences/src/browser/style/index.css Outdated Show resolved Hide resolved
@msujew msujew force-pushed the msujew/align-notifications-preferences branch from 98f6b51 to 21c367b Compare February 10, 2022 23:47
Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, in various themes. 👍

@msujew msujew merged commit 2e0733c into master Feb 15, 2022
@msujew msujew deleted the msujew/align-notifications-preferences branch February 15, 2022 21:20
@github-actions github-actions bot added this to the 1.23.0 milestone Feb 15, 2022
thegecko pushed a commit to ARMmbed/theia that referenced this pull request Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notifications issues related to notifications preferences issues related to preferences ui/ux issues related to user interface / user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants