Skip to content

show the action buttons on top of the modal always by default - #2630

Merged
ChristophWurst merged 1 commit into
masterfrom
enh/noid/modal-timeout
Jun 9, 2022
Merged

show the action buttons on top of the modal always by default#2630
ChristophWurst merged 1 commit into
masterfrom
enh/noid/modal-timeout

Conversation

@szaimen

@szaimen szaimen commented Apr 9, 2022

Copy link
Copy Markdown
Contributor

reason behind this: as noticed in the accessibility workshop, the action buttons of the modal should always visible. Changing the default to -1 will allow to opt-in into the hiding of the action buttons which is in my opinion the better way to do this.

Signed-off-by: szaimen szaimen@e.mail.de

@szaimen szaimen added enhancement New feature or request 3. to review Waiting for reviews design Design, UX, interface and interaction design accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Apr 9, 2022
@szaimen
szaimen requested review from a team, PVince81, skjnldsv and vanpertsch and removed request for a team April 9, 2022 16:42
@skjnldsv

Copy link
Copy Markdown
Contributor

This is a breaking change @jancborchardt

@szaimen szaimen added the 💥 breaking PR that requires a new major version label Apr 10, 2022
@jancborchardt

Copy link
Copy Markdown
Contributor

@szaimen could you add before/after screenshot / gif? I don’t really understand what it does.

@skjnldsv why is this a breaking change?

@szaimen

szaimen commented Apr 12, 2022

Copy link
Copy Markdown
Contributor Author

@szaimen could you add before/after screenshot / gif? I don’t really understand what it does.

Maybe @skjnldsv can? :)

@skjnldsv

Copy link
Copy Markdown
Contributor

@skjnldsv why is this a breaking change?

Because all Modals that were relying on auto-hiding the UI will now have to manually specify the timeout.

@szaimen

szaimen commented Apr 27, 2022

Copy link
Copy Markdown
Contributor Author

@jancborchardt these navigation items of the modal:
image
hide by default after 3s if you don't move your mouse which is bad for mobile and accessibility wise.

After this change they are always visible by default if the developer did not specify a timeout which will hide these items again after a specific timeout...

Hope that is clear now? :)

@jancborchardt

Copy link
Copy Markdown
Contributor

Which of our modals would still use auto-hide here then, if any?

Cause as it came up in the accessibility workshop I would say we should remove the auto-hiding altogether. The close button should always be visible, and if a name is needed that one should also be shown (or not set in the first place), but not hide.

This should not be configurable by individual developers.

@szaimen

szaimen commented Apr 29, 2022

Copy link
Copy Markdown
Contributor Author

This should not be configurable by individual developers.

So then shall I remove this option?

cc @skjnldsv

@szaimen

szaimen commented May 3, 2022

Copy link
Copy Markdown
Contributor Author

ping @skjnldsv
for me it is fine to only change the default for now. WDYT? :)

@szaimen
szaimen requested review from a team and marcoambrosini and removed request for a team May 19, 2022 16:49
@skjnldsv

Copy link
Copy Markdown
Contributor

I'm following Jan's decision :)
If the workshop suggested we should remove, let's set the default to disabled or remove the feature

@szaimen
szaimen force-pushed the enh/noid/modal-timeout branch from 2461559 to cda6d9e Compare May 20, 2022 09:30
@szaimen

szaimen commented May 20, 2022

Copy link
Copy Markdown
Contributor Author

If the workshop suggested we should remove, let's set the default to disabled or remove the feature

I've already set the default to disabled but tried to remove it now in the latest commit. Please suggest what is your choice. Depending on that I'll revert the last commit again.

This is now best reviewed like this: https://github.com/nextcloud/nextcloud-vue/pull/2630/files?diff=unified&w=1

@szaimen
szaimen force-pushed the enh/noid/modal-timeout branch from 32c623e to e1ea93b Compare May 20, 2022 09:43
@raimund-schluessler raimund-schluessler added this to the 6.0.0 milestone May 26, 2022
@szaimen
szaimen force-pushed the enh/noid/modal-timeout branch from e1ea93b to e3ae1dc Compare June 9, 2022 14:21

@ChristophWurst ChristophWurst left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 🐘

please squash

Signed-off-by: szaimen <szaimen@e.mail.de>
@szaimen
szaimen force-pushed the enh/noid/modal-timeout branch from e3ae1dc to b59221e Compare June 9, 2022 14:23
@szaimen

szaimen commented Jun 9, 2022

Copy link
Copy Markdown
Contributor Author

please squash

done

@szaimen
szaimen requested review from skjnldsv and removed request for skjnldsv and vanpertsch June 9, 2022 14:32
@ChristophWurst
ChristophWurst merged commit 122f371 into master Jun 9, 2022
@ChristophWurst
ChristophWurst deleted the enh/noid/modal-timeout branch June 9, 2022 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities 💥 breaking PR that requires a new major version design Design, UX, interface and interaction design enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants