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

show and hide actionmenu elements depending selection and copied elem… #962

Closed
wants to merge 1 commit into from

Conversation

erral
Copy link
Member

@erral erral commented Apr 2, 2020

…ents

Fixes #961

@ionlizarazu

@mister-roboto
Copy link

@ionlizarazu you need to sign the Plone Contributor Agreement in order to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html

@mister-roboto
Copy link

@erral thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@erral erral closed this Apr 2, 2020
@erral erral reopened this Apr 2, 2020
@mister-roboto
Copy link

@erral thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@erral
Copy link
Member Author

erral commented Apr 3, 2020

@jenkins-plone-org please run jobs

@jensens
Copy link
Member

jensens commented Apr 8, 2020

I am not sure if hiding is the best solution, or if it is better to grey out/ disable the buttons.

Opinions?

@erral
Copy link
Member Author

erral commented Apr 10, 2020

Yeah, it should be something that does not allow to make any action when clicking on them, hiding is one option.

@jensens
Copy link
Member

jensens commented Apr 14, 2020

@thet @agitator - any opinions on this?

@thet
Copy link
Member

thet commented Apr 24, 2020

I think disabling would be less disturbing instead of hiding/showing depending on the state. It probably also makes it harder to orientate in the UI when buttons disappear on different places.
On the other side hiding cleans up the UI.
If disabling the buttons is also an option accessibility wise, then I'd go for disabling.

There are already buttons disabled (should be at least), including when pasting and nothing was selected:
Button initialization:

setupButtons: function() {

Paste button disable/undisable:
togglePasteBtn: function(){

ButtonGroup: https://github.com/plone/mockup/blob/65b92f2178df68013ccccf7bdd4354add870422d/mockup/js/ui/views/buttongroup.js
ButtonView: https://github.com/plone/mockup/blob/65b92f2178df68013ccccf7bdd4354add870422d/mockup/js/ui/views/button.js

@vincentfretin
Copy link
Member

This was a regression. See my fixes in #996

@vincentfretin vincentfretin deleted the ionlizarazu-a11y-actionmenu-buttons-visibility branch July 5, 2020 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Structure pattern: action buttons should only be present if there is some action to do
6 participants