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

Enable toggling between analyze and BigCZ results #2119

Merged
merged 2 commits into from
Aug 8, 2017

Conversation

caseycesari
Copy link
Member

Overview

Allow the user to switch between the AoI analyze results, and the BigCZ search page. This implementation is based on the analyze and modeling tabs of MMW. Some views, templates, and CSS rules from that implementation were duplicated and adjusted rather than abstracting the MMW
implementation and making it reusable.

Connects #2095

Demo

bigcz1

Notes

As noted in the PR description and commit message, it proved to be more difficult to directly reuse the MMW components to implement this. Instead, I copied and pared down what I needed. In the short term, I think this was a easier, and less complicated approached. However, it may have drawbacks if we need to make similar changes to both apps.

Also, there were a number of CSS quirks that came up when I implemented this. I was able to fix all of them except for the height of the tab content. Currently, it is set optimally for the search results and not the analyze tab. The download button on the analyze tab is currently not completely viewable. I'll create a follow-up issue for this.

Testing Instructions

  • Create or select an AoI
  • Proceed to the search page.
  • Verify that you can toggle between the analyze and data page using the tab buttons.
  • Verify that in MMW, the analyze and model pages have not been impacted by this change.

@rajadain
Copy link
Member

rajadain commented Aug 7, 2017

Taking a look now.

@rajadain
Copy link
Member

rajadain commented Aug 7, 2017

Cannot scroll all the way to the bottom of the Analyze tabs:

2017-08-07 11 27 29

EDIT: Ah I see this has already been pointed out in the Notes section. Consider using the above gif for the separate card for this issue.

@rajadain
Copy link
Member

rajadain commented Aug 7, 2017

This branch has a dark grey buffer at the bottom of the Search page:

image

which does not exist on develop:

image

@rajadain
Copy link
Member

rajadain commented Aug 7, 2017

And opening the filters makes it worse:

image

@rajadain
Copy link
Member

rajadain commented Aug 7, 2017

This may be more of a design task (/cc @jfrankl), but really long area of interest names don't fit in the header:

2017-08-07 11 37 37

In the very least, hovering on it should reveal the full name in a tooltip. We should also remove the clickability, since they don't have a dropdown menu as MMW does.

Also, we have now lost the area of interest size. But that problem exists in MMW too.

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

I'll take another look once the grey box issue has been resolved.

.data-catalog-stage-results-tab-pane {
.tab-content {
// Set a specific height to allow y-scroll:
height: calc(100% - 53px) !important;
Copy link
Member

Choose a reason for hiding this comment

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

This is likely causing the grey box issues, since it may double the work here:

bottom: 54px; // .pager-region height + padding-top

@arottersman
Copy link

Also going to wait for grey box issue resolution to test. Looked over the code — LGTM.

@caseycesari
Copy link
Member Author

Added 72cded2 to fix the gray box issue.

@jfrankl
Copy link
Contributor

jfrankl commented Aug 8, 2017

I am going to spend some time on the design aspects of this later today. The removal of the project saving functionality will require some adjustments.

@rajadain
Copy link
Member

rajadain commented Aug 8, 2017

Taking another look.

Copy link
Member

@rajadain rajadain left a comment

Choose a reason for hiding this comment

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

+1 tested. Can switch back and forth, and the search page works as fully as it did before. I like the approach used, and the code looks pretty good. Nice work!

The cutoff Area of Interest and Analyze tab scrolling issues can be counted as "known issues" for now. We should make cards for them.

@rajadain rajadain removed their assignment Aug 8, 2017
Copy link

@arottersman arottersman left a comment

Choose a reason for hiding this comment

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

+1

Allow the user to switch between the AoI analyze results, and the BigCZ
search page. This implementation is based on the analyze and modeling
tabs of MMW. Some views, templates, and CSS rules from that implementation
were duplicated and adjusted rather than abstracting the MMW
implementation and making it reusable.

Refs #2095
@caseycesari caseycesari force-pushed the cpc/search-analyze-navigation branch from 72cded2 to df4e326 Compare August 8, 2017 17:26
@caseycesari caseycesari merged commit a9475f8 into develop Aug 8, 2017
@caseycesari caseycesari deleted the cpc/search-analyze-navigation branch August 8, 2017 17:27
@caseycesari caseycesari assigned rajadain and unassigned caseycesari and rajadain Aug 8, 2017
@caseycesari caseycesari self-assigned this Aug 8, 2017
@rajadain rajadain mentioned this pull request Oct 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants