Update "used by" display and tooltip styling #26262
Conversation
sgress454
had a problem deploying
to
Docker Hub
GitHub Actions
Failure
| return deviceMapping.slice(1).map((d) => ( | ||
| return deviceMapping.map((d) => ( |
There was a problem hiding this comment.
Show all emails in the tooltip, since the first one may be truncated.
| <span className="device-mapping__primary-user"> | ||
| {email}{" "} | ||
| <span className="device-mapping__source">{`(${source})`}</span> | ||
| </> | ||
| </span> |
There was a problem hiding this comment.
New class to allow for truncating the email w/ ellipses.
| color: $ui-fleet-black-50; | ||
| } | ||
| &__primary-user { | ||
| max-width: 270px; |
There was a problem hiding this comment.
This allows for up to "+ 999 more" without cutting of the "more" text.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #26262 +/- ##
=======================================
Coverage 63.65% 63.66%
=======================================
Files 1631 1631
Lines 156543 156538 -5
Branches 4045 4089 +44
=======================================
Hits 99655 99655
+ Misses 49038 49032 -6
- Partials 7850 7851 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
sgress454
had a problem deploying
to
Docker Hub
GitHub Actions
Failure
| &__source { | ||
| color: $ui-fleet-black-75; | ||
| } | ||
| &__more { | ||
| color: $ui-fleet-black-50; |
There was a problem hiding this comment.
These were not being used because __data isn't a thing in this component as far as I can see. Moved them out from under __data and now the "More" link is grey as intended.
sgress454
changed the title
| let className = "device-mapping__primary-user"; | ||
| if (newDeviceMapping.length > 1) { | ||
| className += " multiple"; | ||
| } |
There was a problem hiding this comment.
We tend to namespace our class names and use the baseClass as the root of the class name. This helps flatten out the scss styles so they are less nested and easier to read. We also use the classnames package to handle conditional classes. This could replace line 132:
const classNames = classnames(`${baseClass}__device-mapping__primary-user`, {
[`${baseClass}__multiple": newDeviceMapping.length > 1
}I know it's a nitpick, and I know we're not doing it everywhere, but I do think it's better to follow the pattern that we are using across the majority of the codebase.
There was a problem hiding this comment.
Updated. The styles were nested under .info-flex but didn't need to be, so this works!
|
Putting this back in draft as we decided in standup to use |
For #25283
Checklist for submitter
changes/,orbit/changes/oree/fleetd-chrome/changes.See Changes files for more information.
Details
This PR updates the styling on the "Used by" line of the host details page in the following ways:
Screenshots
With extra long email:
Tooltip:
With regular email:
With one email:
With one long email:
^ ideally we'd be able to hover over this to get the whole email. The simplest way to do this would be to just always have the tooltip be active all the time, but that's a product decision. I at least have it dedicating more space to the address if there's only one.