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

fix #5659, Unify the icon color settings for Menu and Toolbar, and fixed some strange places #6199

Merged
merged 4 commits into from
Sep 20, 2019

Conversation

BroKun
Copy link
Member

@BroKun BroKun commented Sep 17, 2019

What it does

fix #5659

  • Use the opacity property to display the availability of the menu, which is better for colored icons.
  • Use the same opacity value when disabled.
  • Make sure the bottom tabbar icon is vertically aligned with the text.
  • Improved UI of search-in-workspace
    • Use pointer cursor when replace-all button is available.
    • Search-detail and search-header are horizontally aligned.
    • Use absolute position to make the clickable area of the toggle button larger.

How to test

  1. The menu bar should look the same.
  2. Use a colored icon in the menu bar, and the icon color should change if the menu is disabled.
  3. The icons in the bottom tab-bar should always be vertically aligned with the text.
  4. search-in-workspace
    1. The disabled icons should look the same grayscale.
    2. Input box are horizontally left aligned.
    3. The show-detail button is always clickable.
    4. When the replace-all button is enable, the cursor should be pointer.

Review checklist

  • as an author, I have thoroughly tested my changes and carefully followed the review guidelines
  • I have thoroughly tested my changes in Chrome 76.0.3809.132 and FireFox 69.0 on macOS 10.14.5.

Reminder for reviewers

Signed-off-by: Brokun <brokun0128@gmail.com>
@BroKun BroKun changed the title Gh 5659 fix #5659, Unify the icon color settings for Menu and Toolbar, and fixed some strange places Sep 17, 2019
@akosyakov akosyakov added the shell issues related to the core shell label Sep 18, 2019
@jbicker
Copy link
Contributor

jbicker commented Sep 18, 2019

These are very good improvements. Thank you!
But I agree with @akosyakov that we should have a css variable for the 'disable' opacity as he mentioned here.

Signed-off-by: Brokun <brokun0128@gmail.com>
- Use pointer cursor when replace-all button is available.
- Search-detail and search-header are horizontally aligned.
- Use absolute position to make the clickable area of the toggle button larger.

Signed-off-by: Brokun <brokun0128@gmail.com>
Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

code changes looks good to me, please rely on @jbicker's approve for testing

Copy link
Contributor

@jbicker jbicker 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 too!

@akosyakov
Copy link
Member

@BroKun please merge

@akosyakov akosyakov merged commit 71eb000 into eclipse-theia:master Sep 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shell issues related to the core shell
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui/ux][icon] Unify the icon color settings for Menu and Toolbar
3 participants