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

Fix invalid rendering of meeting and proposal body texts #9764

Merged
merged 10 commits into from
Oct 26, 2022

Conversation

ahukkanen
Copy link
Contributor

🎩 What? Why?

Currently meeting and proposal body rendering can be broken as explained at #9763. This fixes the described issue.

📌 Related Issues

Testing

  • Create a meeting from admin panel
  • Add a list to the meeting description from the WYSIWYG editor
  • Go to see the meeting page
  • Check the page source
  • See that there are no extra <br> tags within the meeting description list
  • Repeat the same process for admin created proposal

@ahukkanen ahukkanen added module: proposals module: meetings type: fix PRs that implement a fix for a bug labels Aug 26, 2022
@andreslucena
Copy link
Member

Code wise, everything looks 👌🏽

Just a minor change.

With this text:

Selection_307

Before it was rendered like this (with the bug):

Selection_303

With this PR:

Selection_304

I think we should add some padding at the bottom (with CSS or however you prefer). Can you do that please?

Copy link
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.

I've left a comment with a minor change, can you check it out whenever you have time? Thanks

@ahukkanen
Copy link
Contributor Author

Displayed list styles should be now fixed with the latest commits. I also added a margin bottom to the lists when in editor because it bothers me (and some other editors judging that they add extra paragraphs) that the next paragraph is attached to the list above it.

While doing this (and merging with develop), I noticed we've broken the sub-lists when re-editing the lists. Reported that as a separate issue at #9866. It does not seem to be related to this PR.

@andreslucena
Copy link
Member

There are some broken related specs, can you check them out @ahukkanen 🙏🏽?
There's also another failing (from decidim-assemblies) that will be fixed if you merge with develop (related to #9863). Can you merge please?

Copy link
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.

Can you check out the failing specs 🙏🏽? Also, you'll need to merge with develop to fix the one failing from assemblies. Thanks

@ahukkanen
Copy link
Contributor Author

Done @andreslucena

ahukkanen and others added 3 commits October 25, 2022 13:48
Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
Copy link
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.

👍🏽

@andreslucena andreslucena merged commit a647e9e into decidim:develop Oct 26, 2022
@ahukkanen ahukkanen deleted the fix/9763 branch October 26, 2022 08:39
entantoencuanto added a commit that referenced this pull request Oct 26, 2022
* develop: (35 commits)
  Install turbo-rails (#9881)
  Fix conference invitations (#9664)
  Fix invalid rendering of meeting and proposal body texts (#9764)
  Make documentation site work with multiple versions (#9917)
  Bump versions on install docs (#9916)
  Standardize CSV import formats and fix private users CSV import with invalid file (#9627)
  Fix: The i18n locales selector is showing a dropdown with 3 languages (#9902)
  Make Scopes field in debates translatable (#9903)
  Make ToS agreement translatable (#9909)
  Fix issues with a11y specs (#9929)
  Remove invitations badge (#9906)
  Make initiatives order translatable (#9905)
  Add missing active actions on admin navigation menu (#9904)
  Fix user sign up with invalid name (#9896)
  Remove duplication of LastActivity queries (#9895)
  Rename IgnoredMethods to AllowedMethods in Rubocop configuration (#9893)
  Exclude malformed file from codeclimate configuration (#9910)
  Fix correct resource linking for amendments (#9887)
  Fix superposition in admin's error forms (#9871)
  Add missing i18n key in Initiatives (#9892)
  ...
entantoencuanto added a commit that referenced this pull request Oct 31, 2022
* develop: (36 commits)
  Fix proposal etiquette and length validator with base64 images (#9639)
  Install turbo-rails (#9881)
  Fix conference invitations (#9664)
  Fix invalid rendering of meeting and proposal body texts (#9764)
  Make documentation site work with multiple versions (#9917)
  Bump versions on install docs (#9916)
  Standardize CSV import formats and fix private users CSV import with invalid file (#9627)
  Fix: The i18n locales selector is showing a dropdown with 3 languages (#9902)
  Make Scopes field in debates translatable (#9903)
  Make ToS agreement translatable (#9909)
  Fix issues with a11y specs (#9929)
  Remove invitations badge (#9906)
  Make initiatives order translatable (#9905)
  Add missing active actions on admin navigation menu (#9904)
  Fix user sign up with invalid name (#9896)
  Remove duplication of LastActivity queries (#9895)
  Rename IgnoredMethods to AllowedMethods in Rubocop configuration (#9893)
  Exclude malformed file from codeclimate configuration (#9910)
  Fix correct resource linking for amendments (#9887)
  Fix superposition in admin's error forms (#9871)
  ...
Quentinchampenois pushed a commit to Quentinchampenois/decidim that referenced this pull request Nov 23, 2022
* Remove the double sanitization from the meeting description

* Fix the rendering of the safe content

* Add proper styles for the quill lists

* Fix rich text editor specs after class name change

* Revert the edit spec change which was already correct

* Fix typos in specs

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>

* Remove ql-reset-decidim class

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: meetings module: proposals type: fix PRs that implement a fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

<br> tags added in lists
2 participants