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

Fix semantic commands enabled status #14664

Merged

Conversation

fcollonval
Copy link
Member

References

Fixes #14658

Code changes

  • Fix isEnabled for file editor semantic commands
  • Switch notebook toolbar button to use the notebook commands; before by using a semantic command, if the focus switches in a console for example, some actions would still appear enabled
  • Add caption to customized toolbar buttons to customized the button tooltip

User-facing changes

Notebook toolbar buttons status are consistent

Backwards-incompatible changes

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@fcollonval
Copy link
Member Author

bot please update documentation snapshots

@github-actions
Copy link
Contributor

Documentation snapshots updated.

@fcollonval fcollonval force-pushed the fix/14658-command-button-not-updated branch from 87dda4c to 80b9a73 Compare June 13, 2023 12:35
@fcollonval
Copy link
Member Author

bot please update documentation snapshots

@github-actions
Copy link
Contributor

Documentation snapshots updated.

@krassowski
Copy link
Member

before by using a semantic command, if the focus switches in a console for example, some actions would still appear enabled

But is this the expected behaviour? Could these be enabled (rather than disabled) but possibly greyed out as "inactive"? My problem with the buttons being disabled is that it is now a two-step action to run a cell in another notebook (first focus the notebook, then click on the button).

two-step

Even though other UI elements e.g. the "add cell" button are always active:

but-adding-cell-works

I think most desktop UI does not require focusing first to click a button. As an example, the window initially in the background on the screencast has the buttons greyed out (inactive) but when I click on "X" it just works (it is not disabled):

window-buttons

@fcollonval
Copy link
Member Author

Thanks for the feedback @krassowski - I agree with you it will be more consistent to still have the button as enabled. One possible solution would be to return true for isEnabled if the command is in the toolbar bar (we already have a argument toolbar: true for tuning the icon and label). But this is may be not a strong rule. What do you think?

@krassowski
Copy link
Member

Yes, I think this makes sense - subsequently we could apply a different kind of "grey out" style to all toolbar buttons by conditioning on the document not having an "active" class (we don't have a class like that on document widgets - only on tab bar, which is a related UX issue).

@fcollonval
Copy link
Member Author

bot please update documentation snapshots

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023

Documentation snapshots updated.

@fcollonval fcollonval force-pushed the fix/14658-command-button-not-updated branch from de3ca39 to d6656eb Compare August 3, 2023 11:55
@fcollonval fcollonval merged commit bddc378 into jupyterlab:main Aug 3, 2023
77 checks passed
@fcollonval fcollonval deleted the fix/14658-command-button-not-updated branch August 3, 2023 12:57
@fcollonval
Copy link
Member Author

@meeseeksdev please backport to 4.0.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment