-
Notifications
You must be signed in to change notification settings - Fork 68
Simplify the consent screen #5310
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
Conversation
Deploying matrix-authentication-service-docs with
|
| Latest commit: |
e50a716
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b75ee32b.matrix-authentication-service-docs.pages.dev |
| Branch Preview URL: | https://quenting-simpler-consent-scr.matrix-authentication-service-docs.pages.dev |
96a7152 to
fbf5fbf
Compare
reivilibre
left a comment
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.
mostly good, trusting the screenshots for style decisions :-)
| // Fetch informations about the user. This is purely cosmetic, so we let it | ||
| // fail and put a 1s timeout to it in case we fail to query it | ||
| // XXX: we're likely to need this in other places | ||
| let localpart = &session.user.username; | ||
| let display_name = match tokio::time::timeout( | ||
| Duration::from_secs(1), | ||
| homeserver.query_user(localpart), | ||
| ) |
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.
hehehe. It works I guess
crates/templates/src/functions.rs
Outdated
| /// Filter which computes a hash between 1 and 6 of an input string, similar to | ||
| /// compound-web's useIdColorHash |
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.
similar, or giving identical results?
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.
Identical; fixed in 4eb8543
| {{ scopes }} | ||
| {% endcall %} | ||
| #} | ||
| {% macro unsafe_scopes(scopes) -%} |
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.
given the complexity of the macro, please put a few input/output examples in the comment
...though maybe we should register a Jinja2 filter instead, | filter_prefixes([...]) and have it in Rust code?
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.
Fixed in e26a7f3
templates/components/scope.html
Outdated
| Please see LICENSE files in the repository root for full details. | ||
| -#} | ||
|
|
||
| {# Macro to remove 'unsafe' scope from a scope list. Usage: |
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.
...I think it's actually removing the safe ones but preserving the unsafe?
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.
Fixed in e26a7f3
| </h1> | ||
|
|
||
| <div class="session-card my-4"> | ||
| <p class="text [&>span]:whitespace-nowrap [&>span]:text-[var(--cpd-color-text-link-external)]"> |
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.
...do I want to ask what [&>span]:whitespace-nowrap is? Or [&>span]:text-[var(--cpd-color-text-link-external)] for that matter?
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.
Don't worry about it 🫶
It's some tailwind magic for direct children of that node which are spans. Plus arbitrary values around brackets
generates
.\[&>span\]:whitespace-nowrap > span {
whitespace: nowrap;
}
.\[&>span\]:text-\[var(--cpd-color-text-link-external)\] > span {
color: var(--cpd-color-text-link-external);
}
Can be reviewed commit by commit
A few before/after screenshots: