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

Replace privacy policy page. #79

Merged
merged 383 commits into from
Aug 5, 2023
Merged

Replace privacy policy page. #79

merged 383 commits into from
Aug 5, 2023

Conversation

Jonas-Sander
Copy link
Collaborator

@Jonas-Sander Jonas-Sander commented Dec 26, 2021

Replace old privacy policy pages with a new markdown-based, multi-layout privacy policy page.

This PR will not update the privacy policy itself, only the presentation.

Note: This PR originally had code for loading the privacy policy asynchronously (it wasn't loaded from the internet, but all the code for loading indicators, errors, etc. was 90% there). I removed it for now, to have less complex code as we don't need it yet. The deleted code: 7eb67fb3fd4694e1cf4505cc12df821198ea7f22

Desktop / Narrow layout
(Updated: "Weitere Optionen" in the lower-left is now centered)

privacy_policy_x264_30.mp4

Mobile layout

Onboarding Default
grafik grafik
Dark mode Light mode
grafik grafik
grafik grafik
grafik grafik

Privacy policy with Subchapters (just for demonstration purposes)

2.Sharezone.Web-App.-.Google.Chrome.2023-05-08.20-12-55.mp4

Tidbits to remeber:

  • Expansion behavior is different on layouts with bottom sheets in contrast to the desktop layout with the fixed table of contents on the side. On the latter manually expanded sections will stay expanded until they are manually collapsed again, on the former they will always automatically collapse again as soon as they are not read anymore.

TODOs:

  • A section should already be marked as "currently reading" when the heading is at the top of the page not only after we scrolled past it (because if we use the TOC to navigate to a section it will position the heading at the top of the page - also its just nicer in general).
  • Fix Anchor for "11. Verarbeitung des gewählten Account-Typs und des Bundeslandes" not working (maybe more). The generated ID by the Flutter markdown plugin is different from the Id we use in the text. Seems to happen for Anchors with Sonderzeichen/Umlaute?
  • flutter: 'package:sharezone/onboarding/sign_up/pages/privacy_policy/src/table_of_contents/currently_reading.dart': Failed assertion: line 104 pos 14: 'oldVisibleHeadings.length == 1': is not true.
  • Depending on the Platform (and maybe even input?) we should have different Padding for the ui elements, especially the table of contents section texts. On e.g. an iPad you would need to press onto a specific section with your finger and thus need more space to hit it easily and not accidentally another section below or above. On desktop since you have your cursor one might leave less space as its easier to press the right section. We might use Theme.visualDensity - should be additionally look at the type of input used? See also: https://stackoverflow.com/questions/63327808/in-flutter-how-can-i-check-if-a-mouse-device-or-a-touch-device
  • Privacy Policy Section shouldn't stay expanded when opened manually in bottom sheet layout
  • When manually expanding subsections in the BottomSheet they should close automatically again when navigating to one (maybe simple auto open/close behavior with no permanent manual expansion?)
  • Change our designs and implementation so that we don't take consent. See:
    I changed the implementation but didn't change the design.
  • TOC fade at the bottom should be invisible if we scrolled to the bottom
  • Implement downloading as a PDF
  • Dynamically load privacy policy and PDF download link from endpoint
  • Test case: If loading privacyPolicy fails then show button to open up PDF version via web browser
  • Opening TOC bottom sheet - bottom sheet should always open up to about 3/4 of screen (currently only goes to fixed height i think, ignoring screen size).
  • E/FA ( 3642): Name is too long. Type, maximum supported length, name: event, 40,
    ui_privacy_policy_text_scaling_factor_changed
  • Highlight Color of Dark-Mode Selector looks kinda weird (selected item should still have same border, bad contrast in dark mode)
  • 14. Instance ID subheadings (e.g. Firebase Cloud Messaging) have to much space to the left ( subheadings of e.g. 8. Account, Nickname und Passwort are more on the left, like they should)
  • PDF download button doesn't work an Android
  • Space beneath divider (all layouts?)
    image
  • Disabling the fade effect when scrolling to the end of the toc heading list at the bottom is still quite abrupt
  • If you reduce the width of the window the padding around the TOC should be reduced until a "minumum" padding. After that width of the main text should be reduced. If we have much space available we can have more padding than the minumum padding for the TOC to make it look nicer.
  • When opening the toc bottom sheet and one of the last sections / chapters is currently read then the toc section list bounces inside the bottom sheet.
  • Open TOC button not vertically centered
    image
  • Widget test that no error is thrown on different layouts when displaying privacy policy loading placeholder (also test download PDF is not tappable?)
  • Back button at the top should be at the very left in desktop layout.
  • Widget test for bottom sheet opening / choosing section / it closing.
  • Widget test in different layouts loading state (toc cant be opened / placeholder shown, etc).
  • Test that the correct layouts appear in the different window sizes
  • Delete code regarding loading privacy policy.
  • Split up classes in common.dart into files.
  • Divider on desktop layout not visible?
  • "Datenschutzerklärung" heading is not always centered in desktop layout.
  • Remote Config value ("kill switch") that disables the internal privacy policy parsing / display function and instead displays a button / dialog to open the privacy policy in a website / webview
  • Replace "Akzeptieren" of the privacy policy in onboarding with just "forward" (as "Akzeptieren" is wrong and might get us in legal trouble)
  • Navigating to 5b. will highlight wrong subsection (next heading is already in threshold.) We should scroll the selected to expactly the threshold I guess?
  • Should the metadata be included in the actual markdown legal document or be added on the client so we can change the way of presentation later?
  • Document that files in cloud storage need metadata edited to be directly downloaded instead of displayed in browser
  • Think about if different expansion behaviors are really necessary for mobile and desktop
  • Add loading state (GrayShimmer?). Disable buttons while loading.
  • Add loading error state (add button to go to website version of privacy policy?).
  • Add displaying / rendering error state.
  • This error came on mobile:
I/flutter (19159): ----------------FIREBASE CRASHLYTICS----------------
I/flutter (19159): Invalid argument(s): Can't find section with id information-ber-die-verarbeitung-personenbezogener-daten inside allSectionsFlattend ([inhaltsverzeichnis, 1-wichtige-begriffe, 2-geltungsbereich, 3-verantwortlichkeit-und-kontakt, 4-hosting-backend-infrastruktur-und-speicherort-fr-eure-daten, 5-deine-rechte, a-recht-auf-auskunft, b-recht-auf-berichtigung, c-recht-auf-lschung, d-recht-auf-einschrnkung-der-verarbeitung, e-recht-auf-widerspruch, f-recht-auf-widerruf, g-recht-auf-datenbertragbarkeit, h-recht-auf-beschwerde, 6-eure-kontaktaufnahme, 7-unser-umgang-mit-euren-daten, 8-account-nickname-und-passwort, a-registrierung-mittels-anonymen-accounts, b-registrierung-mit-e-mail-adresse--passwort-oder-googleapple-sign-in-ab-einem-alter-von-16-jahren-und-lter, 9-verarbeitung-der-ip-adresse, 10-speicherdauer-und-speicherfristen, 11-verarbeitung-des-gewhlten-account-typs-und-des-bundeslandes, 12-anonyme-statistische-auswertung-der-app-nutzung, 13-push-nachrichten, 14-instance-id, firebase-cloud-messaging, firebase-cr
I/flutter (19159): #0      _PrivacyPolicyViewport._computeCurrentlyRead
package:sharezone/…/table_of_contents/currently_reading.dart:147
I/flutter (19159): #1      _PrivacyPolicyViewport.currentlyReadSectionOrNull
package:sharezone/…/table_of_contents/currently_reading.dart:131
I/flutter (19159): #2      new CurrentlyReadingSectionController.<anonymous closure>
package:sharezone/…/table_of_contents/currently_reading.dart:64
I/flutter (19159): #3      ChangeNotifier.notifyListeners
package:flutter/…/foundation/change_notifier.dart:308
I/flutter (19159): #4      ValueNotifier.value=
package:flutter/…/foundation/change_notifier.dart:412
I/flutter (19159): #5      new DocumentSectionController.<anonymous closure>
package:sharezone/…/table_of_contents/table_of_contents_controller.dart:81
I/flutter (19159): #6      ChangeNotifier.notifyListeners
package:flutter/…/foundation/change_notifier.dart:308
I/flutter (19159): #7      ValueNotifier.value= (package:flu
I/flutter (19159): ----------------------------------------------------
  • Text should be scrollable if your cursor is anywhere inside the main text area. Not above the text.
  • Integration/Widget tests that scroll the policy from start to finish and ensures that all sections in the TOC get highlighted (in the correct order)
  • Add tickets for an automated testing pipeline:
    - If changes are made in the privacy policy display code
    - If a new privacy policy is written
    (Should check all privacy policies with all versions of the privacy policy display code)
  • Separate privacy policy code in its own package? (For the purposes above) --> Maybe even own repo because we might use it in the website as well?
  • Appearance dialog settings apply to privacy policy page but not to the dialog. If e.g. textScaling is changed the text inside the dialog doesn't change, same for brightness (although the latter is correct if you close and open the dialog again after changing it)
  • Fade-In Color for highlighted toc section looks weird in light mode (starts with gray?)
  • Version should be somewhere in the text
  • Jump to section by clicking on the title, only expand it (no jumping) by clicking on the arrow icon next to it.
  • Fix the closing animation (should close smoothly and not just disappear)
  • Future: Change languages
  • Future: Compare versions
  • Future: Definitions of word can be seen next to a defined word by clicking on ? symbol to the right of the word

Need Feedback on:

  • Animation (duration) for expansion / collapse of section headings in table of contents

Nice to have but not worth it right now (add as issues?):

  • Add widget tests for expansion behavior change when changing layouts (e.g. from desktop to tablet layout)

Stuff that doesn't work 100% as I want it but I can't or don't want to fix:

  • Desktop TOC bottom fade will not always be disabled when at the bottom if a section expanded/collapses. (Scroll to bottom and expand and then collapse a section. At first the fade will be disabled but after expanding/collapsing it will be there altough it shouldn't be).

@Jonas-Sander
Copy link
Collaborator Author

Current first prototype:

Screenshot_1640550445

@Jonas-Sander
Copy link
Collaborator Author

Jonas-Sander commented Jan 15, 2022

TODO: Fix
image

@Jonas-Sander
Copy link
Collaborator Author

Jonas-Sander commented Jan 15, 2022

Relative anchors don't work right now in the markdown package:
flutter/flutter#81777

So that means the sidebar with the table of content probably won't be added for now. I'm also considering to leave out the table of content inside the text as it, well... won't work. Will probably be more confusing if we left it in.
Otherwise there are only like 2 relative anchors inside the text so that should be alright.

@github-actions
Copy link

github-actions bot commented Apr 5, 2022

Visit the preview URL for this PR (updated for commit 661ddd7):

https://sharezone-test--pr79-privacy-policy-rewor-4w6bbtfu.web.app

(expires Wed, 27 Jul 2022 16:08:50 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@Jonas-Sander Jonas-Sander marked this pull request as draft April 12, 2022 17:29
@Jonas-Sander Jonas-Sander changed the title [WIP] Privacy policy rework Privacy policy rework Apr 12, 2022
Comment on lines 130 to 158
Flexible(
child: ConstrainedBox(
constraints: BoxConstraints(maxWidth: 600),
child: Padding(
padding: const EdgeInsets.only(top: 20),
child: Column(
mainAxisAlignment: MainAxisAlignment.center,
crossAxisAlignment: CrossAxisAlignment.center,
children: const [
_PrivacyPolicyHeading(),
Padding(
padding: const EdgeInsets.symmetric(
horizontal: 20, vertical: 8.0),
child: _PrivacyPolicySubheading(),
),
Divider(),
Flexible(child: _PrivacyPolicyMarkdown()),
Divider(),
Padding(
padding:
const EdgeInsets.symmetric(vertical: 20),
child: _AcceptionButtons(),
)
],
),
),
),
),
],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nilsreichardt
I want to constrain the privacy policy text to a maxWidth (wich works via ConstrainedBox ) but I want to center the text (which does not work).
Wrapping the Flexible in a Center or Alignement does not work. Changing the mainAxisAlignement of the Row which wrapps these widgets isn't a soluton (i think), since I want to lay out the table of contents at the left and only center the text.

image

Copy link
Member

Choose a reason for hiding this comment

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

You need to wrap the ConstrainedBox into a Expanded widget. Therefore, takes the widget the complete remaining space of the parent Row (_TableOfContents and VerticalDivider takes only what the need). Now to center the ConstrainedBox inside the Expanded widget, you need to wrap it with a Center widget.

Row(
  children: [
    _TableOfContents(),
    VerticalDivider(),
    Expanded(
      child: Center(
        child: ConstrainedBox(
          constraints: BoxConstraints(maxWidth: 600),
          child: Padding(
            padding: const EdgeInsets.only(top: 20),
            child: Column(
              mainAxisAlignment: MainAxisAlignment.center,
              crossAxisAlignment: CrossAxisAlignment.center,
              children: const [
                _PrivacyPolicyHeading(),
                Padding(
                  padding: const EdgeInsets.symmetric(
                      horizontal: 20, vertical: 8.0),
                  child: _PrivacyPolicySubheading(),
                ),
                Divider(),
                Flexible(child: _PrivacyPolicyMarkdown()),
                Divider(),
                Padding(
                  padding: const EdgeInsets.symmetric(
                      vertical: 20),
                  child: _AcceptionButtons(),
                )
              ],
            ),
          ),
        ),
      ),
    ),
  ],

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Wtf, I'm sure I tried this exact combination before and for me if I used Expanded my BoxConstraints were just ignored 🤔🤔

@Jonas-Sander
Copy link
Collaborator Author

Would be really helpful if I could build for macOS since Web doesn't support Hot reload. Currently I'm running everything in a iPad Simulator on my MacBook since it's close to a desktop regarding the form factor. I can't really resize it though - I can only scale it (which doesn't affect the app since the scaling only changes the size of the iPad on my MacBook, not the size that is used for rendering on the iPad).

@nilsreichardt #64 is blocked on #153, correct? Can I help there somehow?

@nilsreichardt
Copy link
Member

@nilsreichardt #64 is blocked on #153, correct? Can I help there somehow?

Yes, this is correct. The new Flutter version broke our theme and this need to be fixed. Maybe I can fix it in today in evening or you will do it before

@Jonas-Sander
Copy link
Collaborator Author

Jonas-Sander commented May 27, 2022

@Jonas-Sander
Copy link
Collaborator Author

@nilsreichardt Ready for review! :D

The only thing I'll still have to do is to open issues for some of the stuff that's written in the PR (some of the unchecked checkboxes).

Good luck haha
...good thing I didn't absolutely over-engineer this solution 😅😅

@Jonas-Sander Jonas-Sander marked this pull request as ready for review May 8, 2023 18:28
@Jonas-Sander Jonas-Sander changed the title Privacy policy rework Replace privacy policy page. May 8, 2023
Copy link
Member

@nilsreichardt nilsreichardt left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Awesome PR 🚀🚀🚀

It's too big to make really a review. If you think, everything is fine. You can merge it 👍

image

Maybe it would look better to also center the "Weitere Optionen" since the button are also centered. Or setting the alignment of the button to left. But I think mixing it looks a bit weird.

@nilsreichardt
Copy link
Member

nilsreichardt commented Jul 16, 2023

Btw: Would be really nice to have something like this as a package. Even as an internal package would be nice

@Jonas-Sander
Copy link
Collaborator Author

Maybe it would look better to also center the "Weitere Optionen" since the button are also centered. Or setting the alignment of the button to left. But I think mixing it looks a bit weird.

Yes, of course, I changed that 👍
Idk why I left it like this

Jonas-Sander and others added 2 commits August 4, 2023 19:01
@Jonas-Sander
Copy link
Collaborator Author

Btw: Would be really nice to have something like this as a package. Even as an internal package would be nice

Will do that in another PR

@Jonas-Sander Jonas-Sander added this pull request to the merge queue Aug 4, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 4, 2023
@Jonas-Sander Jonas-Sander added this pull request to the merge queue Aug 5, 2023
Merged via the queue into main with commit 9ff554d Aug 5, 2023
@Jonas-Sander Jonas-Sander deleted the privacy-policy-rework branch August 5, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Changing, updating, adding or removing one or more dependencies. feature: authentification Logging in/out (anonymous, sign-in with X, etc.) and registration. feature: homework feature: onboarding The steps (setting username, courses, etc.) after creating a new account. legal Regarding Licenses, Policy updates, Warnings to users (that might cause trouble if not there) etc. testing ui / ux user: pupil / student w: onboarding/privacy-policy-page
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

2 participants