Skip to content

Fix character counter for the WYSIWYG editor#9680

Merged
andreslucena merged 3 commits intodecidim:developfrom
mainio:fix/wysiwyg-character-counter
Sep 15, 2022
Merged

Fix character counter for the WYSIWYG editor#9680
andreslucena merged 3 commits intodecidim:developfrom
mainio:fix/wysiwyg-character-counter

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

🎩 What? Why?

The character counter is currently broken for the WYSIWYG editor I believe after the screen reader updates at #9009.

This makes the character counter work again with the WYSIWYG editor.

📌 Related Issues

Testing

  • Enable rich text editor for participants
  • Create a new proposal
  • See that the character counter is working correctly in the view

Note that the "empty" field character counting is a bit buggy in Quill.js (which is the rich text editor). It automatically injects one paragraph to the text with a line break in it, so I believe it is because of this reason why it calculates the "empty" input as 2 characters.

@ahukkanen ahukkanen added module: core type: fix PRs that implement a fix for a bug labels Aug 12, 2022
Crashillo
Crashillo previously approved these changes Aug 18, 2022
Copy link
Copy Markdown
Contributor

@Crashillo Crashillo left a comment

Choose a reason for hiding this comment

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

Since we've in mind the jQuery deprecation, it would be useful to replace some basic functions with vanilla js. A gradual migration.

@andreslucena
Copy link
Copy Markdown
Member

@ahukkanen it's probably something related with my assets, but I tried rebuilding them and still see the same: while adding characters, the #proposals_body_characters_sr changes to NaN (as in At least 15 characters, NaN characters left).

Can you check this out to confirm if you see this same behavior, please? I can reproduce it on Firefox and Chrome. It seems like if a focus out of the window, the NaN changes to the correct count.

wordcounter-nan.mp4

@ahukkanen
Copy link
Copy Markdown
Contributor Author

@andreslucena No, it was actually a bug in the character counter. Not related to this PR but anyways a bug.

It's fixed now.

While checking that I also noticed that the WYSIWYG editor allows more characters than the maximum amount of characters defined for the body text which causes the character counter to go to negative values. But this is another issue that should be addressed separately from this PR.

@andreslucena
Copy link
Copy Markdown
Member

While checking that I also noticed that the WYSIWYG editor allows more characters than the maximum amount of characters defined for the body text which causes the character counter to go to negative values. But this is another issue that should be addressed separately from this PR.

This "while checking that bug I found another bug" remind me of this classic short video 😅

I agree, let's split it on multiple PRs, so we can move forward with this one.

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Tried it out locally and work as expected. It also fixed the screen reader bug. Two for the price of one!! 😄

@andreslucena andreslucena merged commit c289cf5 into decidim:develop Sep 15, 2022
@ahukkanen ahukkanen deleted the fix/wysiwyg-character-counter branch September 15, 2022 08:40
entantoencuanto added a commit that referenced this pull request Sep 15, 2022
* develop: (24 commits)
  Add develop index to the documentation (#9666)
  Fix initiatives components (#9633)
  Fix conference speaker avatars (#9643)
  Update `rokroskar/workflow-run-cleanup-action` GitHub action to v0.3.3 (#9750)
  Fix character counter for the WYSIWYG editor (#9680)
  Fix posting comments before the initial load has run (#9614)
  Fix parallel tests port in use (#9661)
  Split parallel test coverage reports into their own folders (#9686)
  Improve admin panel user experience regarding title links and order of actions (#9496)
  Fix title and description too long in initiatives spec sometimes (#9648)
  Fix API GraphiQL system spec with newer ChromeDriver (#9642)
  Add missing character on code block (#9798)
  Fix hidden error messages on the registration form (#9625)
  Add documentation about configuring ActiveStorage / dynamic file uploads (#9777)
  Add documentation section about customizing cells (#9622)
  Fix hashtags not recognized at the beginning of the string (#9616)
  Fix version pages showing a HTTP 500 error when the version does not exist (#9615)
  Fix multitenant organizations stats cache (#9605)
  Prevent the account edit route through Devise (#9611)
  Fix iframe disabling producing invalid HTML (#9685)
  ...
@paulinebessoles
Copy link
Copy Markdown
Contributor

@ahukkanen do you also plan to backport this feature in 0.26?

@ahukkanen
Copy link
Copy Markdown
Contributor Author

@paulinebessoles Probably not because we have changed the character counter in 0.27, so this cannot be directly backported to 0.26. It would have to be re-implemented for 0.26.

eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
* Fix character counter for the WYSIWYG editor

* Fix announce threshold calculation bug reported in the review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants