- 
 
- 
  Notifications
 You must be signed in to change notification settings 
- Fork 512
ATL-1128: make the new tab button easier to click #258
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
Conversation
 
 @ubidefeo
 
 ubidefeo
 
 
 
 left a comment
 
 
 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the clickable area is fine, but I get the contextual menu with a weird X offset
Screenshot 2021年03月23日 at 15 15 43 
the clickable area is fine, but I get the contextual menu with a weird X offset
Screenshot 2021年03月23日 at 15 15 43
@ubidefeo this issue should be unrelated, plus, I'm not able to reproduce it. Did you resize the window or moved to another virtual desktop?
@kittaakos did you see this issue before?
I have experienced the same, which was related to changing the interface scale (see ATL-1080). @ubidefeo do you have any modifications to the File > Preferences > Interface Scale setting?
did you see this issue before?
No, I have not but the issue must be either somewhere here:
arduino-ide/arduino-ide-extension/src/browser/contributions/sketch-control.ts
Lines 59 to 62 in 3fb087f
or was already fixed in Theia: eclipse-theia/theia#9082. Maybe, there is nothing to do other than updating to the most recent next Theia. We're still on eclipse-theia/theia@c9db9754 
did you see this issue before?
No, I have not but the issue must be either somewhere here:
arduino-ide/arduino-ide-extension/src/browser/contributions/sketch-control.ts
Lines 59 to 62 in 3fb087f
anchor: {x: parentElement.getBoundingClientRect().left,y: parentElement.getBoundingClientRect().top + parentElement.offsetHeight}or was already fixed in Theia: eclipse-theia/theia#9082. Maybe, there is nothing to do other than updating to the most recent
nextTheia. We're still on eclipse-theia/theia@c9db975
let's address this in a separate PR then, thanks
84603a2 to
 a8df82e  
 Compare
 
 
 
 
 
 ubidefeo
 
 
 
 commented
 Mar 25, 2021 
 
 
 
@kittaakos @fstasi 
what should be done for this one, then?
what should be done for this one, then?
I would do the followings:
- rebase ATL-1118: Add keymaps customization support #231 and merge if it was approved,
- rebase this PR from main, start a build see if the context menu bug still occurs.- If no, we're good. The issue has been fixed in Theia. We can merge this PR.
- If yes, we need to fix it either as a follow-up or with this PR.
 
 
 
 
 ubidefeo
 
 
 
 commented
 Mar 25, 2021 
 via email 
 
 
 
Can you please rebase the PR from the main? Also, please add the following to fix the issue when there are multiple "sketch-control" toolbar items. Locating them via their ID is flawed. Thank you!
diff --git a/arduino-ide-extension/src/browser/contributions/sketch-control.ts b/arduino-ide-extension/src/browser/contributions/sketch-control.ts index 6e4010c..db0aa48 100644 --- a/arduino-ide-extension/src/browser/contributions/sketch-control.ts +++ b/arduino-ide-extension/src/browser/contributions/sketch-control.ts @@ -6,6 +6,7 @@ import { ContextMenuRenderer } from '@theia/core/lib/browser/context-menu-render import { Disposable, DisposableCollection } from '@theia/core/lib/common/disposable'; import { URI, SketchContribution, Command, CommandRegistry, MenuModelRegistry, KeybindingRegistry, TabBarToolbarRegistry, open } from './contribution'; import { ArduinoMenus } from '../menu/arduino-menus'; +import { EditorWidget } from '@theia/editor/lib/browser'; @injectable() export class SketchControl extends SketchContribution { @@ -24,15 +25,28 @@ export class SketchControl extends SketchContribution { registerCommands(registry: CommandRegistry): void { registry.registerCommand(SketchControl.Commands.OPEN_SKETCH_CONTROL__TOOLBAR, { isVisible: widget => this.shell.getWidgets('main').indexOf(widget) !== -1, - execute: async () => { + execute: async widget => { this.toDisposeBeforeCreateNewContextMenu.dispose(); const sketch = await this.sketchServiceClient.currentSketch(); if (!sketch) { return; } + let target: HTMLElement | undefined = undefined; + if (widget instanceof EditorWidget) { + const tabBar = this.shell.getTabBarFor(widget); + if (tabBar) { + const elements = document.querySelectorAll(`#${SketchControl.Commands.OPEN_SKETCH_CONTROL__TOOLBAR.id}`); + for (const element of Array.from(elements)) { + if (!target) { + if (tabBar.node.contains(element)) { + target = element as HTMLElement; + } + } + } + } + } - const target = document.getElementById(SketchControl.Commands.OPEN_SKETCH_CONTROL__TOOLBAR.id); - if (!(target instanceof HTMLElement)) { + if (!target) { return; } const { parentElement } = target;
The active area for the new tab button is very small.
image
This PR expands the active area by moving the padding from the container of the button to the button itself.
Padding changed because of the actual size of the chevron icon in the button.
Fixes #97
Jira Task