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

Use CSS to control ViewContainerPart toolbar visibility #9935

Merged
merged 1 commit into from
Aug 24, 2021

Conversation

colin-grant-work
Copy link
Contributor

@colin-grant-work colin-grant-work commented Aug 20, 2021

What it does

This PR supersedes #8967, as it contains alternative logic for updating the toolbar.

At the moment, we have quite an elaborate routine for managing the visibility of the toolbars on ViewContainerParts. The results differ from VSCode in a couple of ways:

  • The toolbar disappears if a context-menu is opened using the ... more icon.
  • The toolbar isn't always visible if the widget is focused.

How to test

  1. Test different states of widget focus and hovering.
    • The toolbar should never be visible if the ViewContainerPart is collapsed
    • The toolbar should be visible if the widget is focused
    • The toolbar should be visible if you hover over the widget
    • The toolbar should be visible if a context menu is opened from the toolbar

Review checklist

Reminder for reviewers

Signed-off-by: Colin Grant [email protected]

@colin-grant-work colin-grant-work force-pushed the feature/cleaner-toolbar-management branch from be4cbc0 to 199c072 Compare August 20, 2021 02:38
@colin-grant-work colin-grant-work marked this pull request as ready for review August 20, 2021 02:41
@colin-grant-work colin-grant-work added the ui/ux issues related to user interface / user experience label Aug 20, 2021
Copy link
Member

@msujew msujew 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 me:

  • Focusing the container part always shows the toolbar ✔️
  • Hovering inside the container part always shows the toolbar ✔️
  • A collapsed container part never shows the toolbar ✔️
  • Using the ellipsis menu still shows the toolbar ✔️
  • Everything works as expected when there's only a single container part inside the view ✔️

@vince-fugnitto vince-fugnitto self-requested a review August 20, 2021 12:17
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good and work well for me 👍

  • the toolbar should never be visible if the ViewContainerPart is collapsed
  • the toolbar should be visible if the widget is focused
  • the toolbar should be visible if you hover over the widget
  • the toolbar should be visible if a context menu is opened from the toolbar
  • confirmed that executing toolbar items, and menu items in the ... (more) context-menu works well

@vince-fugnitto vince-fugnitto added the toolbar issues related to the toolbar label Aug 20, 2021
@colin-grant-work colin-grant-work force-pushed the feature/cleaner-toolbar-management branch from 199c072 to 468490b Compare August 23, 2021 15:02
@colin-grant-work colin-grant-work force-pushed the feature/cleaner-toolbar-management branch from 468490b to 350fbca Compare August 23, 2021 15:03
@colin-grant-work colin-grant-work merged commit 471f10a into master Aug 24, 2021
@colin-grant-work colin-grant-work deleted the feature/cleaner-toolbar-management branch August 24, 2021 13:25
@github-actions github-actions bot added this to the 1.17.0 milestone Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
toolbar issues related to the toolbar ui/ux issues related to user interface / user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants