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

[ui] Remove animation from task logs sidebar #15146

Conversation

philrenaud
Copy link
Contributor

@philrenaud philrenaud commented Nov 4, 2022

Resolves #15113

Context: While our other sidebar portals (Evaluations, Services) are parent-contextual ("expand info on this already-existing child of my route"), Task logs are different: they fetch out-of-context information (logs for a task) from several different places (including a client index, or a job index).

Because of this:

  • We opted to make the sidebars conditional elements within each of the <TaskSubRow> components, conditional upon that row being selected (having "View Logs" clicked)
  • This means that the animation treatment was different for each; instead of depending on an "open" status, we always animated it in/out when it showed up in the DOM

This presents a problem: because the context can change (ie: job stops, client stops), the task log context also changes and gets "repainted" in the DOM. This caused the sidebar to "jitter" and fire its animation again as if it were a brand new element that needed to close and re-open again.

There is another way to solve this, involving moving the portal architecture for this to the application-level template (the html equivalent of global scope), but I've opted not to do this for maintainability reasons.

As such: hasta la vista to the sidebar animation. It now just pops open or closed upon clicking "View Logs"

image

@philrenaud philrenaud added the backport/1.4.x backport to 1.4.x release line label Nov 4, 2022
@philrenaud philrenaud self-assigned this Nov 4, 2022
@github-actions
Copy link

github-actions bot commented Nov 4, 2022

Ember Asset Size action

As of 8c27032

Files that got Smaller 🎉:

File raw gzip
nomad-ui.css -185 B -85 B

Files that stayed the same size 🤷‍:

File raw gzip
nomad-ui.js 0 B 0 B
vendor.js 0 B 0 B
vendor.css 0 B 0 B

Copy link
Member

@tgross tgross left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@mikenomitch mikenomitch left a comment

Choose a reason for hiding this comment

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

👍 feels good to me on vercel

@github-actions
Copy link

github-actions bot commented Nov 4, 2022

Ember Test Audit comparison

main 8c27032 change
passes 1425 1425 0
failures 0 0 0
flaky 0 0 0
duration 10m 05s 536ms 10m 02s 193ms -03s 343ms

@philrenaud philrenaud merged commit 6a75882 into main Nov 7, 2022
@philrenaud philrenaud deleted the 15113-ui-task-sidebar-slide-out-closes-and-re-opens-any-time-a-task-state-changes branch November 7, 2022 15:11
@github-actions
Copy link

github-actions bot commented Mar 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 Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/1.4.x backport to 1.4.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ui] Task Sidebar slide-out: closes and re-opens any time a task state changes
3 participants