Skip to content
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

Update client list to collapse statuses #5789

Merged
merged 45 commits into from
Jun 19, 2019
Merged

Conversation

backspace
Copy link
Contributor

@backspace backspace commented Jun 6, 2019

This adjusts the clients list to be more compact. The former columns for eligibility and draining are merged with the status and called “state”.

no-hover-overflow

This collapses the eligibility and draining attributes into
the status. Draining takes precedence, then (in)eligibility;
if neither of those is true, the status displays.

Still to come: restoring colouring shown for draining and
ineligibility, sorting, facets.
Using assert.ok without ember-qunit-nice-errors calls for
explanatory strings.
This is hackish but functional, can be refined or abandoned
depending on feedback.
Now that the ineligible and draining columns are collapsed
into the status column, it seems a bit confusing to have
them in a separate facet. But is it also confusing to have
them undifferentiated in the status facet? This is easily
removed, if so.
0.75 wasn’t working because it was from td rather than
tbody td. Without the unsightly calc, the background
overlaps with the border of the next row. 🤨
I guess it’s okay…???
@backspace backspace changed the title Update client list to collapse statuses Update client list to collapse statuses and show full name on hover Jun 10, 2019
@backspace backspace marked this pull request as ready for review June 10, 2019 21:15
@backspace backspace requested review from DingoEatingFuzz and a team June 10, 2019 21:17
@backspace
Copy link
Contributor Author

Direct link to the relevant route for easier viewing

Copy link
Contributor

@DingoEatingFuzz DingoEatingFuzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! Despite all my comments, I think this is at least 90% there.

ui/app/controllers/clients/index.js Outdated Show resolved Hide resolved
ui/app/controllers/clients/index.js Outdated Show resolved Hide resolved
ui/app/styles/core/table.scss Outdated Show resolved Hide resolved
ui/app/styles/core/table.scss Outdated Show resolved Hide resolved
ui/app/styles/core/table.scss Outdated Show resolved Hide resolved
ui/app/templates/components/client-node-row.hbs Outdated Show resolved Hide resolved
ui/mirage/scenarios/default.js Outdated Show resolved Hide resolved
ui/tests/acceptance/clients-list-test.js Show resolved Hide resolved
ui/tests/acceptance/clients-list-test.js Outdated Show resolved Hide resolved
ui/tests/acceptance/clients-list-test.js Show resolved Hide resolved
@DingoEatingFuzz
Copy link
Contributor

@backspace, do you mind also updating the PR description to include a screenshot of the change in action? I try to include one in every visual change for historian reasons 📚

This ensures the revealable-on-hover content stays atop
the content with an attached tooltip.
This ensures the revealable-on-hover content stays atop
other revealable content.
This doesn’t belong in this PR, just trying it for now.
@backspace
Copy link
Contributor Author

backspace commented Jun 11, 2019

Sensible practice; will do after the appearance is fully settled. What do you think about the shadow showing within the cell when there’s no overflow? I’m wondering if there’s a way to only show it when it’s actually too wide 🤔

hover-progress

ETA hmm well the GIF is weirdly truncated but there’s a brief glimpse where you can see that the shadow shows in a weird place when hovering over a shorter-named cell.

There’s a jitter again in the production deployment that’s not present for me locally, so I’ll have to figure that out too.

The width of the shadow-blocker for cells that don’t actually
overflow is hardcoded to the 300px width, so it doesn’t make
sense for it to be applied generally.
Maybe this was only helping in Chrome Canary? 🤪
“to right” isn’t supported in Safari, the alpha needs to be
a fraction of 1, and it should be fading to white not black!
@backspace
Copy link
Contributor Author

As a result of this flaw that shows with short names/addresses and a narrower window, I’m removing the “show full name on hover” aspect of this 😢 I’ll have to take another approach.

narrower-overflow-crisis

@backspace backspace changed the title Update client list to collapse statuses and show full name on hover Update client list to collapse statuses Jun 14, 2019
@backspace backspace merged commit 237c406 into master Jun 19, 2019
@notnoop notnoop deleted the f-ui/long-client-names branch June 22, 2019 06:11
@github-actions
Copy link

github-actions bot commented Feb 8, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants