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

terminal: fix split-terminal visibility #12626

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Conversation

vince-fugnitto
Copy link
Member

What it does

The commit fixes #12357 which caused the split-terminal toolbar item from leaking to other views incorrectly when a terminal was currently visible.

Related #12358.

Screenshots

Before:

Screen Shot 2023-06-15 at 1 17 15 PM

After:

Screen Shot 2023-06-15 at 1 14 43 PM

How to test

  1. start the application
  2. open a terminal - confirm the split-terminal toolbar item is present and works
  3. confirm the split-terminal toolbar item is not present in other views when a terminal is visible
  4. confirm the split-terminal toolbar item is present for subsequent terminals

Review checklist

Reminder for reviewers

The commit fixes the terminal `split-terminal` toolbar item from appearing in other
views when the terminal is active.

Signed-off-by: vince-fugnitto <[email protected]>
@vince-fugnitto vince-fugnitto added the terminal issues related to the terminal label Jun 15, 2023
@vince-fugnitto vince-fugnitto self-assigned this Jun 15, 2023
@vince-fugnitto
Copy link
Member Author

cc @rschnekenbu do you mind reviewing, the changes from #12358 were still problematic on master.

@rschnekenbu
Copy link
Contributor

@vince-fugnitto, sure, i'll run the review today.

Copy link
Contributor

@rschnekenbu rschnekenbu left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@vince-fugnitto vince-fugnitto requested a review from msujew June 21, 2023 13:32
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 to me 👍 I can confirm that the command doesn't show up on unrelated widgets and is always visible on the terminal widget.

dankeboy36 added a commit to dankeboy36/arduino-ide that referenced this pull request Jun 27, 2023
@vince-fugnitto vince-fugnitto merged commit 507bdc8 into master Jun 27, 2023
@vince-fugnitto vince-fugnitto deleted the vf/split-terminal branch June 27, 2023 18:48
@github-actions github-actions bot added this to the 1.39.0 milestone Jun 27, 2023
dankeboy36 added a commit to dankeboy36/arduino-ide that referenced this pull request Jun 28, 2023
Removed the toolbar contribution from the UI.

Ref: eclipse-theia/theia#12626/
Signed-off-by: dankeboy36 <[email protected]>
kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Jul 5, 2023
Removed the toolbar contribution from the UI.

Ref: eclipse-theia/theia#12626/
Signed-off-by: dankeboy36 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
terminal issues related to the terminal
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Remove invasive Split Terminal action in the toolbars
3 participants