-
-
Notifications
You must be signed in to change notification settings - Fork 768
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: Do not add advanced prop on submit #436
fix: Do not add advanced prop on submit #436
Conversation
In my opinion, it looks better without the default option and the red color helps it stand out so it's easier to tell which attribute was added. I think a simpler solution to the issue would be to just add the Feel free to let me know what you think. |
I hope I got your right, I have tried the following and it didn't work: - <input class="btn" type="submit" value="Open Permalink">
+ <input class="btn" type="button" value="Open Permalink"> With the change above, pressing One of the below changes works fine:
diff --git a/src/demo/js/script.js b/src/demo/js/script.js
document.addEventListener("click", () => preview.update(), false);
+document.querySelector("form").addEventListener("keydown", (event) => {
+ if (event.key === 'Enter') event.preventDefault();
+});
diff --git a/src/demo/index.php b/src/demo/index.php
<h2>Properties</h2>
- <form class="parameters">
+ <form class="parameters" onkeydown="return event.key != 'Enter';">
<label for="user">Username<span title="required">*</span></label> @DenverCoder1 please let me know what you think of it. Which one is more preferable or maybe you have a different vision of it. Thanks. |
The reason it adds a property is because Enter triggers a form submit so it finds the first button that does not have the Since it is not meant to submit the form, the add property button and remove property buttons should have Prevent add property buttons from submitting: --- a/src/demo/index.php
+++ b/src/demo/index.php
- <button class="plus btn" onclick="return preview.addProperty();">+</button>
+ <button type="button" class="plus btn" onclick="return preview.addProperty();">+</button> Prevent remove property buttons from submitting: --- a/src/demo/js/script.js
+++ b/src/demo/js/script.js
minus.innerText = "−";
+ minus.setAttribute("type", "button");
minus.setAttribute("data-property", propertyName); Not required -- Prevent "open permalink" from submitting and signify semantically that copy is not a submit: --- a/src/demo/index.php
+++ b/src/demo/index.php
- <input class="btn" type="submit" value="Open Permalink">
+ <input class="btn" type="button" value="Open Permalink">
- <button class="copy-button btn tooltip" onclick="clipboard.copy(this);" onmouseout="tooltip.reset(this);" disabled>
+ <button type="button" class="copy-button btn tooltip" onclick="clipboard.copy(this);" onmouseout="tooltip.reset(this);" disabled> Let me know if you think this makes sense. I've tested it and it works for me. |
33d6e4d
to
50773aa
Compare
Co-authored-by: DenverCoder1 <jonah@freshidea.com>
Co-authored-by: DenverCoder1 <jonah@freshidea.com>
Co-authored-by: DenverCoder1 <jonah@freshidea.com>
50773aa
to
736b022
Compare
@DenverCoder1 thank you for the detailed feedback! 🚀 I appreciate that. I skipped adding
Let me know what you think. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your continued contributions, looks great 🎉
Description
Fixes #432
Type of change
How Has This Been Tested?
composer test
Checklist:
Screenshots
N/A