-
Notifications
You must be signed in to change notification settings - Fork 1
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
Make message editable #15
Conversation
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.
Hey Niels. Thank you for working on this, I left some changes because this is not how we offer text editing.
1088b42
to
51d8a87
Compare
@senadir I've updated this PR according to your feedback and double-checked that the functionality is working as expected, both with Gutenberg activated and deactivated. I'm looking forward to receiving your feedback on this update. |
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.
Hey Niels, so far this is looking good and in the correct path, however, there are a lot of things left that I pointed out.
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.
This is looking good, I left a feedback to replace viewSwitcher with the newer version, and a note about notice attribute, but all is fine!
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.
This is looking good! Thank you for working on this.
We have a lot of reused coded from blocks here, we should look (later) into exporting it.
28fedbf
to
c8f91b3
Compare
Thanks for your review and approving these changes.
Absolutely. The two biggest parts I reused are the |
Fixes #4
Note
In i1 of this block, the merchant must be able to edit the messages for insufficient and sufficient cart totals. This PR adds this functionality.
To enable merchants to adjust the message for insufficient cart totals, e.g.
Spend only $35.00 more to get free US shipping!
, according to their needs, I initially created two textareas. One textarea for the message before the remaining value and another one for the message after the remaining value. To keep the editing of the messages simple, I decided against that solution and created just one textarea to adjust the message for insufficient cart totals. This message includes the placeholder%s
. In the help text, I highlighted the importance of this placeholder.I created the two tags Insufficient-totals-with-separate-textareas and Insufficient-totals-with-combined-textarea so that both versions can be checked during the code review.
Testing instructions
Insufficient totals message
andSufficient totals message
.Free shipping from
value to see both messages.