Fix character counter with emoji picker close to maximum characters#8916
Conversation
ahukkanen
left a comment
There was a problem hiding this comment.
I would like to suggest one structural change in the component so that we don't create too strong interconnection with the different JS components. I think the best way would be for each component to work as isolated as possible.
What I mean by this is that if we decide to re-write the emoji picker, we wouldn't need to do any changes in the character counter in that situation.
Based on this idea, I added one change request below.
deactivate and change color of the button
ahukkanen
left a comment
There was a problem hiding this comment.
Can you take a look at the comment I left above and also could you fix the eslint errors please? 🙏
ahukkanen
left a comment
There was a problem hiding this comment.
Thanks for the changes @eliegaboriau !
Getting closer, just few more change requests. 🙏
|
I hope it's ok now ! |
ahukkanen
left a comment
There was a problem hiding this comment.
One more small thing below @eliegaboriau. 🙏
Also, could you please correct the ESLint errors. You should be able to install the ESLint extension to your code editor but if you don't know how, you can also run this command in the Decidim root to see the errors:
npm run lintThere was a problem hiding this comment.
LGTM, thanks for the great work and the test case you added!
I also tested this locally and it works as expected.
It won't affect my approval but as a general advice I suggest not doing any unnecessary changes, such as the formatting of the import and export statements that you did. This makes your PRs much easier to review and also makes less places for merge conflicts if other people are working on the same files.
Just to take note for the future!
|
Thanks for the PR @eliegaboriau!! Can you please backport to v0.26-stable 🙏🏽? |
Here it is : #9012 |
🎩 What? Why?
PR to make the emoji button disabled when there's not enough character left
📌 Related Issues
Testing
📋 Checklist
docs/.