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

Hidden tabs can no longer be clicked #9125

Merged
merged 1 commit into from
Feb 25, 2021

Conversation

andrewcarlotti
Copy link
Contributor

@andrewcarlotti andrewcarlotti commented Feb 25, 2021

Fixes #5199, Fixes #6100

What it does

This fixes #5199 and #6100, whereby clicking on the toolbar could trigger a switch to a tab hidden 'behind' the toolbar.

The issue arose because the phosphor.js tabbar event handler only looks at the coordinates of click events, so mustn't be called if the click event is outside the tabbar (but on the hidden tabbar overflow). However, the Theia tabbar click handler was originally passing all click events to the phosphor.js event handler. #5201 partially fixed this by filtering out clicks on the icon buttons, but failed to filter out clicks elsewhere in the toolbar.

How to test

Open enough tabs to overflow the space in the tab bar. Click on elements in the toolbar (including the output select menu and space around the edge of the toolbar).

Review checklist

Reminder for reviewers

Fixes eclipse-theia#5199 and eclipse-theia#6100

Signed-off-by: Andrew Carlotti <andrew.carlotti@arm.com>
@vince-fugnitto vince-fugnitto added bug bugs found in the application output issues related to the output toolbar issues related to the toolbar labels Feb 25, 2021
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.

Thank you @andrewcarlotti I confirmed that the issue related to the output toolbar is resolved with the following pull-request.

@vince-fugnitto
Copy link
Member

@kittaakos would you mind giving a quick review, we can potentially fit it in for today's release.

@kittaakos
Copy link
Contributor

would you mind giving a quick review, we can potentially fit it in for today's release.

Today won't work for me; I have another release also.

Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

Code LGTM and it fixes the issue. CI failure is unrelated.

@paul-marechal paul-marechal merged commit 88f8757 into eclipse-theia:master Feb 25, 2021
@paul-marechal paul-marechal added this to the 1.11.0 milestone Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application output issues related to the output toolbar issues related to the toolbar
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Toolbar of Output View can't be selected Preview / Open Source Button changes tab
4 participants