-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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, deployments] Deployment history on steady state #17167
Conversation
67e7ab3
to
3d2cf93
Compare
Ember Asset Size actionAs of c90694d Files that got Bigger 🚨:
Files that got Smaller 🎉:
Files that stayed the same size 🤷:
|
Ember Test Audit comparison
|
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.
LGTM
export default class JobStatusDeploymentHistoryComponent extends Component { | ||
@service notifications; | ||
|
||
@tracked isHidden = this.args.isHidden; |
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.
note
: If you ever invoke the component with the value of this argument as a variable instead of a hard coded boolean value, this property will not reactively update based on the value of that variable changing. Using a reactive getter will remove the possibility of a bug like this.
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.
@ChaiWithJai Yep, started with that, but my toggle would cease to work in this case. I could have an "in-between" variable to use for toggling but that felt like things would be getting out of sorts.
Adds a toggle to show/hide deployment history, and adds task event history to steady-state job status panels
Resolves #17159