-
Notifications
You must be signed in to change notification settings - Fork 10
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: make the privacy dialogue fully visible on smaller screens #359
Conversation
With a maxHeight of 95vh, we now automatically get a scroll bar if needed, and the Opt out of/in to analytics button is always visible. Also up the width from 50vw to 60vw, which again is nicer on screens smaller than large monitors. Fixes #352
Review changes with SemanticDiff. Analyzed 5 of 5 files. Overall, the semantic diff is 81% smaller than the GitHub diff.
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Took the tour, it appears to work well. Thanks!
>demonstration site here</a | ||
>. In any case, you can opt out entirely by clicking the "Opt Out" button | ||
below (you can always opt back in later). | ||
</p> | ||
</div> | ||
|
||
<div mat-dialog-actions style="display: flex; justify-content: space-evenly"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, I agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I'm not sure which element that would belong on...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And, fixed.
Based on Del's PR review comments
With a maxHeight of 95vh, we now automatically get a scroll bar if needed, and the Opt out of/in to analytics button is always visible.
Also up the width from 50vw to 60vw, which again is nicer on screens smaller than large monitors.
PR Goal?
Make the Opt out of / in to analytics button visible on smaller screens, and add a scroll bar so the whole policy can be read on them.
Fixes?
Fixes #352
Feedback sought?
sanity checking, more or less just rubber stamping for this one
Priority?
medium-high
Tests added?
no
How to test?
click on Privacy and shrink your viewport to see the Opt in/out button is still usable and the full text can still be accessed.
Confidence?
high
Version change?
no