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

Add Monitor Tab #2632

Merged
merged 3 commits into from
Feb 1, 2018
Merged

Add Monitor Tab #2632

merged 3 commits into from
Feb 1, 2018

Conversation

rajadain
Copy link
Member

Overview

Adds Monitor tab to the Analyze stage. Updates styling to match the mockup. The Monitor tab currently has no behavior, which will be added in #2622, mostly by reusing the models and copying a lot of the views from BiG-CZ.

Connects #2620

Demo

2018-01-31 10 48 31

Notes

As part of an earlier attempt I had the full search available in the monitor tab (see #2620 (comment)), I ultimately decided against using the BiG-CZ views as is for this. This decision is motivated by two constraints:

  1. Existing BiG-CZ behavior should not change at all
  2. Monitor tab should fit in well with MMW

There are things we'll need to do for 2 which would contradict 1. For example, in BiG-CZ when switching between the Data and Analyze tabs the map state does not change. Whereas in MMW it will. Another issue is that much of BiG-CZ's search UI is absolutely positioned, which does not readily fit in with the altered sizes in MMW. We should create a UI using flexbox and other more modern methods so that the contents fit and work nicely.

Thus, I created a /monitor package in the JavaScript source folder, to be populated with views and templates used in the monitor tab in MMW, and will leave BiG-CZ untouched.

Testing Instructions

  • Checkout this branch and bundle
  • Go to :8000/ and draw / select a shape
  • Proceed to Analyze. Ensure you see two tabs above, "Analyze" and "Monitor"
  • Switch between the tabs and ensure that scroll state is preserved
  • Ensure the styling matches the mockups in the original issue
  • Ensure that all the specifications in the original issue have been satisfied

Previously the data catalog collection would be initialized
in the data catalog controller, which was appropriate since
that was the only place it would be used.

However, since now we'll need to use it in at least one more
place, namely the upcoming Monitor tab, we move it to App,
using the same pattern as for Analyze Collection.

This is a pure refactor, and does not affect BiG-CZ behavior
at all.
Adds a tab selection header to the top of the analyze sidebar,
and a placeholder for where the monitor contents will go.

Adjusts styling to better match the mockup by making the top
tab bar grey and everything else white, and accounts for it
when calculating the tab content height so that scrolling
works correctly.
@rajadain
Copy link
Member Author

Talked to @mmcfarland about this a little bit. The conclusion from that conversation was:

  • We are not under strict orders to "not change" BiG-CZ
  • We should aim to be efficient and not introduce duplications that will be hard to maintain

Given this, I think it will be reasonable to reuse as many of the data_catalog views as possible (such as search card, search details, etc), and just style them differently. The parent view will have to be different to accommodate the new tab header and such, but all the inner views should be reused.

@arottersman
Copy link

Taking a look now

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.

Tested in our supported browsers and looking good. Everything in the issue is in. Should this view also appear in Model > Analyze tab mode?

@@ -0,0 +1,34 @@
.monitor-window {
padding: 15px;

Choose a reason for hiding this comment

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

Consider setting this to 1rem so it lines up with the AOI title
screen shot 2018-01-31 at 4 12 24 pm

@arottersman arottersman assigned rajadain and unassigned arottersman Jan 31, 2018
@rajadain
Copy link
Member Author

Should this view also appear in Model > Analyze tab mode?

Yes, to be addressed in #2621.

@jfrankl
Copy link
Contributor

jfrankl commented Feb 1, 2018

Looks good!

This matches the mockup and demos the tab switching correctly.
It currently has no behavior.

In order to not affect BiG-CZ, it is better to create a new
set of views under /monitor rather than reuse and change the
ones under /data_catalog. We will use the same models though,
and much of the functionality will be copied over, so it is
not going to be a completely new effort.

All the styling uses flexbox.
@rajadain
Copy link
Member Author

rajadain commented Feb 1, 2018

Thank you for taking a look! Amended the padding. Will merge when green.

@rajadain
Copy link
Member Author

rajadain commented Feb 1, 2018

Jenkins is failing all builds for unrelated reasons (that are being worked on separately). Going to merge this in so that downstream work is unblocked.

@rajadain rajadain merged commit fb9127f into develop Feb 1, 2018
@rajadain rajadain deleted the tt/monitor-add-monitor-tab branch February 1, 2018 18:36
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

3 participants