Skip to content

Fix ToS agreement display#6716

Merged
tramuntanal merged 4 commits intodecidim:developfrom
Dynnammo:fix/user-tos-display
Oct 30, 2020
Merged

Fix ToS agreement display#6716
tramuntanal merged 4 commits intodecidim:developfrom
Dynnammo:fix/user-tos-display

Conversation

@Dynnammo
Copy link
Copy Markdown
Contributor

@Dynnammo Dynnammo commented Oct 21, 2020 •
edited
Loading

🎩 What? Why?

Currently, the Terms of Service (hereafter named TOS) aren't displayed correctly when a user must accept them.

I simply removed the sticky attributes, therefore the content of TOS would be displayed normally, with the form always displayed below.

It's also more GDPR compliant, as it forces users to read Terms of Services before using the platform

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Add documentation regarding the feature
  • Add/modify seeds
  • Add tests
  • Another subtask

📷 Screenshots (optional)

Before (when signed in and ToS have been modified)
image

After (when signed in and ToS have been modified)
image

👻 GIF (optional)

Description

@Dynnammo Dynnammo marked this pull request as ready for review October 26, 2020 08:26
@Dynnammo Dynnammo marked this pull request as draft October 26, 2020 08:26
@Dynnammo Dynnammo marked this pull request as ready for review October 27, 2020 13:20
@Dynnammo
Copy link
Copy Markdown
Contributor Author

Dynnammo commented Oct 27, 2020

I didn't get why the CI codecov is irregular depending on the commits : for some it reaches 100 %, but the variability of this CI step is weird to me, as I didn't change much files in this PR. I would gladly recheck my work in order to make it compliant in the case I missed something on this subject.

@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Oct 28, 2020

Hi @Dynnammo the ToS are also rendered in the sign up form. Can you please add a snapshot so we can check they also render correctly there?

@tramuntanal tramuntanal self-assigned this Oct 28, 2020
@Dynnammo
Copy link
Copy Markdown
Contributor Author

Dynnammo commented Oct 28, 2020 •
edited
Loading

Hey @tramuntanal . ToS on sign up aren't changed, which is basically struggling : the text is not formated, so no more bold/underlined text etc. ,links and embedded videos. Should it be considered as an issue ? In this case, I seems relevant to me to correct it in a separate PR. What do you suggest ?

EDIT : after verification, this behavior is here since 0.18-stable (at least)

Normal display when not signed / already registered
Normal display

But when accessing sign up page, formatting is gone
Sign up page : formatting is gone...

tramuntanal
tramuntanal suggested changes Oct 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

hi @Dynnammo
The PR LGTM but I see you've added 3 new i18n files. I18n files should only be managed by crowdin, can you remove them from the PR please?

  • decidim-elections/config/locales/ar-SA.yml
  • decidim-elections/config/locales/cs-CZ.yml
  • decidim-elections/config/locales/sk-SK.yml

tramuntanal
tramuntanal approved these changes Oct 30, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

Thanks!

@tramuntanal tramuntanal merged commit 84a9231 into decidim:develop Oct 30, 2020
@tramuntanal
Copy link
Copy Markdown
Contributor

tramuntanal commented Oct 30, 2020

@Dynnammo would you like to backport to `release/0.23-stable'?

@verarojman verarojman mentioned this pull request Nov 20, 2020
Closed
verarojman added a commit to SomComuns/decidim-catencomu that referenced this pull request Nov 20, 2020
@verarojman
Fix terms and conditions bug following decidim/decidim#6716
30c9bfd
@paulinebessoles paulinebessoles mentioned this pull request Nov 24, 2020
Closed
@verarojman verarojman mentioned this pull request Nov 26, 2020
Merged
@tramuntanal tramuntanal mentioned this pull request Dec 3, 2020
Merged
tramuntanal pushed a commit that referenced this pull request Dec 3, 2020
@armandfardeau @Dynnammo
Remove sticky from tos agreement (#6716) (#6954)
1299e15 
* Remove sticky from tos agreement

* Modify tests to check visibility of modified TOS

* Remove i18n files

Co-authored-by: Dynnammo <33259633+Dynnammo@users.noreply.github.com>
@tramuntanal tramuntanal mentioned this pull request Dec 3, 2020
Merged
@mrcasals mrcasals mentioned this pull request Dec 21, 2020
Closed
@mrcasals mrcasals changed the title Remove sticky from tos agreement Fix ToS agreement display Feb 26, 2021
@mrcasals mrcasals added module: core type: fix PRs that implement a fix for a bug labels Feb 26, 2021
microstudi pushed a commit to SomComuns/decidim-catencomu that referenced this pull request Jun 16, 2025
@verarojman
Fix terms and conditions bug following decidim/decidim#6716
a70dc2b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tramuntanal tramuntanal tramuntanal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tramuntanal tramuntanal

Labels

in-review module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Dynnammo @tramuntanal @mrcasals