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

[AMBARI-24580] [Log Search UI] Change the fix width and the height for the modal to flexible layout #2251

Merged
merged 3 commits into from
Sep 7, 2018

Conversation

tobias-istvan
Copy link
Member

@tobias-istvan tobias-istvan commented Sep 5, 2018

What changes were proposed in this pull request?

I created a new modal component named as ModalDialogComponent. I kept the ModalComponent still there because I want to change the current modals one by one.

The LogIndexFilter screen was the main reason why we did this change, so that is why I fixed its layout here by changing the Modal to ModalDialog, moving the cluster selection logic into the parent, using the sticky position to have a better UX (except IE 11).

Since I wanted to reuse the data loader icon here, I created a standalone component for this.

I found a typo in a component and I fixed this.

The huge number of files is mainly because the last two changes.

Note: There is a known issue: when you click on the Filter button the first time it does not open, but it will when the filters params are already there. This bug is already fixed within AMBARI-23820 which is almost done.

How was this patch tested?

It was tested heavily manually and by run the unit tests:

PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 271 of 271 SUCCESS (13.135 secs / 12.972 secs)
✨  Done in 42.08s.

Please review Ambari Contributing Guide before opening a pull request.

@asfgit
Copy link

asfgit commented Sep 5, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/3856/
Test FAILed.
Test FAILured.

…r the modal to flexible layout - adding the missing licenses to the new files
@asfgit
Copy link

asfgit commented Sep 6, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/3858/
Test FAILed.
Test FAILured.

@tobias-istvan
Copy link
Member Author

tobias-istvan commented Sep 6, 2018

The test failures have no connection with the changes.

<div *ngIf="showBackdrop" class="modal-backdrop" (click)="onBackdropClick($event)"></div>
<div class="modal-dialog" role="document">
<div class="modal-content">
<div class="modal-header">
Copy link
Contributor

Choose a reason for hiding this comment

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

If there's neither title nor header content nor close button provided, there will be an empty area displayed (whis is the padding of empty modal-header). Don't think it's expected

<div class="modal-body">
<ng-content select="main"></ng-content>
</div>
<div class="modal-footer">
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the same is relevant for footer as well

@asfgit
Copy link

asfgit commented Sep 7, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/3882/
Test PASSed.

@tobias-istvan tobias-istvan merged commit 8289c7f into apache:trunk Sep 7, 2018
@tobias-istvan tobias-istvan deleted the AMBARI-24580-trunk branch September 7, 2018 16:26
vishalsuvagia pushed a commit to vishalsuvagia/ambari that referenced this pull request Oct 1, 2018
…r the modal to flexible layout (apache#2251)

* [AMBARI-24580] [Log Search UI] Change the fix with and the height for the modal to flexible layout

* [AMBARI-24580] [Log Search UI] Change the fix width and the height for the modal to flexible layout - adding the missing licenses to the new files

* [AMBARI-24580] [Log Search UI] Change the fix width and the height for the modal to flexible layout
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants