-
Notifications
You must be signed in to change notification settings - Fork 12
AddressLink formatting fixes #2107
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
Deployed to Cloudflare Pages
|
7e9fd31 to
4f55839
Compare
| <Box | ||
| component="span" | ||
| sx={{ display: 'inline-flex', alignItems: 'center', gap: 3, flexWrap: 'wrap' }} | ||
| > | ||
| <AccountMetadataSourceIndicator source={accountMetadata!.source} /> | ||
| <HighlightedText text={accountName} pattern={highlightPattern} /> ({address}) |
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.
Short version: #2072 didn't create the error, just exposed it by triggering it accidentally. Now I fix both the original problem and the accidental trigger.
Here is the full history.
- There was a error in
AdaptivelyTrimmedAccountLink: under specific conditions (if there is sufficient width to show both account name and address at the same line), it rendered ugly content. - However, this error was not triggered very often, because
AdaptivelyTrimmedAccountLinkwas only used when we were short on space. - However, in Further enhance ROFL app endorsement display #2072, when introducing an option for always enabling the adaptive trimmer in
AccountLink(even if we are on desktop and have lots of space), I have introduced an error by checking the wrong variable in a condition. (I meant to check the new option,alwaysAdapt, but I accidentally wrotealwaysTrimOnTablet.) This resulted in accidentally usingAdaptivelyTrimmedAccountLinkin unintended situations, including situations with lots of available width, therefore triggering this pre-existing, but previously hidden error. - This PR fixes the condition (for selecting the right kind of link), plus it improves both
AdaptivelyTrimmedAccountLinkandDesktopAccountLinkto better handle wrapping / non-wrapping situations, thereby eliminating the original error.
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.
so I'm concerned something else will break.
That's not an unreasonable assumption; that is why I did manual testing by slowly changing the width and checking the behavior on multiple pages. I don't see anything broken.
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.
ohh that makes sense
dde4d93 to
ffaebb2
Compare
- If we show both account name and address, put the addressin parenthesis. - Properly align name and address side by side - More consistent wrapping and gap setting
- Fix reference to wrong variable name, which messed up the table/desktop selection in some situations
ffaebb2 to
833be73
Compare
Small adjustments in AddressLink CSS.
Improves #2096.