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

V8: Do not submit the current content editor when editing a media picker item #7188

Closed

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Nov 20, 2019

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #7185

Description

See #7185 for an issue description. To reproduce it you need a content type where the media picker is not in multipicker mode and is somewhat topmost in the list of property editors.

Here's a GIF of the issue in action:

edit-media-item-before

It is a side effect of turning the buttons into actual tags instead of tags (in an effort to make the media picker more accessible, see #6805). Depending on your property editor order and configuration this means that the browser might see the link picker edit media button as the first element in the form and thus treat it as a submit button.

This PR fixes it by applying an explicit type to the edit button (type="button"), thus explicitly instructing the browser how to interpret it. The prevent-default directive could also have been used here, but the proposed solution leverages the built-in browser functionality and I do think that's preferable.

Here's a GIF of the fix in action:

edit-media-item-after

@MMasey
Copy link
Contributor

MMasey commented Nov 20, 2019

Thanks for picking this up @kjac, the bug was my bad. You may want to add the type=“button” to the delete button too to cover all grounds.

Thanks again

@kjac
Copy link
Contributor Author

kjac commented Nov 20, 2019

Aye. The remove button seems to work just fine also after changing the edit one 👍

@bjarnef
Copy link
Contributor

bjarnef commented Dec 6, 2019

@kjac I think the delete button should have type="button" as well. Without this attribute some browsers has "submit" as default behaviour, while other have "button" as default.

Note how the validation is triggered in the "content" tab, when removing the media. I think the validation only should trigger when saving the node. On load it also seems to show the validation error color on "content" tab shortly until the media has loaded.

@kjac
Copy link
Contributor Author

kjac commented Feb 13, 2020

Fixed in #7651 so I'm closing this PR.

@kjac kjac closed this Feb 13, 2020
@kjac kjac deleted the v8/fix/media-picker-edit-item-saves-content branch February 13, 2020 06:46
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.

Clicking edit on media picker launches the save dialog
3 participants