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

Template widget must handle focus #146

Merged
merged 1 commit into from Jul 21, 2022
Merged

Template widget must handle focus #146

merged 1 commit into from Jul 21, 2022

Conversation

JonasHelming
Copy link
Contributor

fixed #145

Signed-off-by: Jonas Helming jhelming@eclipsesource.com

fixed #145

Signed-off-by: Jonas Helming <jhelming@eclipsesource.com>
@JonasHelming JonasHelming changed the title template widget must handle focus Template widget must handle focus May 5, 2022
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.

@JonasHelming the changes look good, but they do not take effect since we do not use activate when opening the view, see:

execute: () => super.openView({ activate: false, reveal: true })

@JonasHelming
Copy link
Contributor Author

Make sense, but without the fix, I get an error that the widgets was not activated. The first video is the template as is, the second with my fix. I believe this is a regression anyways, because I never experienced this issue in the template before and I use it quite frequently.

withoutfix
withfix

@vince-fugnitto
Copy link
Member

@JonasHelming it does not seem to happen to me at least without the fix, I don't see any code at the moment that uses the activate request for the widget.

focus-2.mov

I did a fresh build, and am using the latest version of the generator. If you can reproduce with a fresh generation can you provide steps as to how you reproduced the warning.

@JonasHelming
Copy link
Contributor Author

I am also on the latest version
I just:

  • generated the widget extension
  • Start the browser app
  • open the template widget (view => name of the widget)
  • focus another view (e.g. the explorer)
  • focus the template widget by clicking on the tab (either the icon in the site bar on the left stack or the tab when in the main stack, both leads to the same result
  • observe the warning the the server console

@jfaltermeier Could you please restest this as well?

@jfaltermeier
Copy link
Contributor

I am also on the latest version I just:

  • generated the widget extension
  • Start the browser app
  • open the template widget (view => name of the widget)
  • focus another view (e.g. the explorer)
  • focus the template widget by clicking on the tab (either the icon in the site bar on the left stack or the tab when in the main stack, both leads to the same result
  • observe the warning the the server console

@jfaltermeier Could you please restest this as well?

With those steps I can reproduce the issue when switching back to the view from the Explorer by clicking on the view's icon in the left tab bar.
However when (re-)opening the view from the menu I did not get the warning (because the command uses activate: false I guess)

I've applied the proposed changes to the generated template view and this fixes the issue.

Should we maybe change the command to use activate: true as well?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generated widget does not correctly accept focus
3 participants