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

Feature/disable preview button during save #11895

Merged
merged 17 commits into from
Sep 13, 2022

Conversation

brianjhanson
Copy link
Contributor

@brianjhanson brianjhanson commented Sep 6, 2022

Description

Disables the preview button while a draft is saving in order to prevent the wrong field layout from showing when the preview opens.

Related issues

Copy link
Contributor

@gcamacho079 gcamacho079 left a comment

Choose a reason for hiding this comment

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

@brianjhanson nice update! Just one change I'd like to suggest:

  • For screen reader accessibility, the aria-disabled attribute is preferred over disabled. It requires the extra step of preventing clicks programmatically, but is otherwise a usability improvement since screen reader users are alerted that the button is there, just inactive at the moment.

I was torn about whether the disabled state should be applied to the button immediately upon saving instead of on Preview click — and having the disablePreviewButton called with the beforeRun event — but I do see the benefit of keeping it clickable while the new layout is loading.

@brianjhanson brianjhanson changed the base branch from develop to 4.3 September 8, 2022 13:20
@brianjhanson
Copy link
Contributor Author

Thanks @gcamacho079! I ended up removing the event listeners when the preview button is disabled but I'm not sure if that's going too far. Let me know if there's a better way to disable programatic clicks.

Copy link
Contributor

@gcamacho079 gcamacho079 left a comment

Choose a reason for hiding this comment

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

Looks good, @brianjhanson. An alternative to adding/removing the click listener would be to immediately exit the openPreview function if the button already has the aria-disabled attribute.

@brandonkelly
Copy link
Member

An alternative to adding/removing the click listener would be to immediately exit the openPreview function if the button already has the aria-disabled attribute.

That feels a little safer to me.

@brianjhanson
Copy link
Contributor Author

@brandonkelly just pushed up that change

# Conflicts:
#	src/web/assets/cp/dist/cp.js
#	src/web/assets/cp/dist/cp.js.map
#	src/web/assets/cp/dist/css/cp.css
#	src/web/assets/cp/dist/css/cp.css.map
[ci skip]
@brandonkelly brandonkelly merged commit be7f814 into 4.3 Sep 13, 2022
@brandonkelly brandonkelly deleted the feature/disable-preview-button-during-save branch September 13, 2022 12:32
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.

3 participants