-
Notifications
You must be signed in to change notification settings - Fork 10
ROFL app details page fixes #1960
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
|
e31b360
to
aa371e6
Compare
aef6d99
to
7c5dd81
Compare
This flags means that we don't really want to see a link, just the label, but we still want to see all the other transformations applied. (This makes it easier to keep things consistent)
7c5dd81
to
3a4ab1e
Compare
These are all optional features, disabled by default.
This fixes title on mobile, when displaying a long address. (The link is used in labelOnly mode, so no actual link is displayed)
3a4ab1e
to
23539cb
Compare
<MuiLink component={RouterLink} to={to}> | ||
{name ? <HighlightedText text={name} pattern={highlightedPart} /> : address} | ||
</MuiLink> | ||
{labelOnly ? ( |
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.
Any alternative ideas for future to avoid every component adding labelOnly mode?
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.
Any alternative ideas for future to avoid every component adding labelOnly mode?
So, the rationale was this:
We want to do a lot of things in our links. (Show a name, or an address, or both; do highlighting based on mouse pointer and search pattern, do shortening, either fixed or adapting length, specific formatting). There are many combinations of this, some is based on configuration, some context dependent, etc.
We want to do all of these consistently both on links and labels. Duplicating the code would suck, so we keep everything together, and use the labelOnly
flag to switching off linking when not needed.
An alternative would be to always have a pair of components, like <WhateverLink>
and <WhateverLabel>
, which would share their internal formatting and other behavior logic, except the link thing itself. Should we try that?
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.
🤷 doesn't sound better
Tweak 1: implement page title shortening in table and mobile mode
Tweak 2: Enable highlighting matching ROFL addresses on mouse hover
(Currently this is only enabled on desktop.)
Tweak 3:
Enable tool-tip on shortened app ids: