Skip to content

Hidden tabs can no longer be clicked#9125

Merged
paul-marechal merged 1 commit intoeclipse-theia:masterfrom
ARMmbed:hidden-tab-clicks
Feb 25, 2021
Merged

Hidden tabs can no longer be clicked#9125
paul-marechal merged 1 commit intoeclipse-theia:masterfrom
ARMmbed:hidden-tab-clicks

Conversation

@alicecarlotti
Copy link
Contributor

@alicecarlotti alicecarlotti 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