[AMBARI-22802] Styling for install wizard #134
Conversation
|
Refer to this link for build results (access rights to CI server needed): |
vivekratnavel
changed the title
jonathan-hurley
requested review from
Jetly-Jaimin,
vivekratnavel,
u39kun,
atkach,
hiveww and
geksilla
ambari-web/app/assets/index.html
Outdated
| $('[data-toggle="tooltip"]').tooltip(); | ||
| //enables tooltips added later | ||
| const body = document.getElementsByTagName('body')[0]; | ||
| const observer = new MutationObserver(() => { |
There was a problem hiding this comment.
It's bad for performance to trigger tooltip on any DOM change, also it may add already removed tooltips(which should be hidden). Use MutationObserver locally with a particular element that needs tooltip.
There was a problem hiding this comment.
Where exactly would you move this code? I already tried putting it in some API hook of the view and it would not work.
There was a problem hiding this comment.
You can put tooltip initialization in didInsertElement of an appropriate view.
There was a problem hiding this comment.
I tried that, but it did not work. I need something that is guaranteed to run after all dynamic items are added to the DOM. didInsertElement() only runs after the template is initially added. If you have, say, table rows that contain tooltips that get added after the initial render, they will not get activated. At least, that was my observation.
There was a problem hiding this comment.
I meant to add MutationObserver in didInsertElement, and it will observe change only inside table or any other parent element.
| @@ -36,7 +36,7 @@ App.ModalPopup = Ember.View.extend({ | |||
| disableSecondary: false, | |||
| disableThird: false, | |||
| primaryClass: 'btn-success', | |||
| secondaryClass: 'btn-default', | |||
There was a problem hiding this comment.
By design secondary button should have "btn-default" class
There was a problem hiding this comment.
Why? It seems like a secondary button should have the btn-secondary class. Otherwise, why is that part of the spec?
There was a problem hiding this comment.
Yeah, we have "btn-secondary" class, change is good.
|
Refer to this link for build results (access rights to CI server needed): |
# Conflicts: # ambari-web/app/styles/application.less # ambari-web/app/styles/wizard.less # ambari-web/app/templates/installer.hbs
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
retest this please |
|
Refer to this link for build results (access rights to CI server needed): |
| @@ -149,8 +149,9 @@ App.WizardStep7Controller = App.WizardStepController.extend(App.ServerValidatorM | |||
| if (!this.get('stepConfigs.length')) return true; | |||
There was a problem hiding this comment.
You can replace all "if"s by one expression joined by "or"
|
Refer to this link for build results (access rights to CI server needed): |
What changes were proposed in this pull request?
Styling for new and updated Install Wizard components. This includes:
How was this patch tested?
All units tests passing:
20297 passing (25s)
125 pending