Review and integrate open250 #274

charlesh88 · 2015-11-10T01:38:43Z

#250 and open250 address three front-end issues:

Tree does not properly display when search bundle is excluded #185 - Allow the tree pane to handle its layout better when bundles are not included, like search.
[Mobile] Suppress Create button in Mobile #157 - Hide the Create button by default while in mobile, but allow devs to re-display it if needed by including a bundle.
View switcher menu not appearing #260 - Fix a regression bug that was preventing the view switcher menu from appearing when invoked from the main object view area.

Author checklist:

Changes address original issue? Y
Unit tests included and/or updated with changes? Y
Command line build passes? Y
Expect to pass code review? Y

open #250 Markup mods in browse.html; Renamed .holder-create-and-search to .holder-treeview-elements; TO-DO: fix search results to scroll properly;

open #250 Significant changes to flex classes, markup in browse.html and search.html to support better flex layout; Search results now scrolls properly; Significant refactoring and cleanups in search and _search.scss;

open #250 Fixed margin/spacing problems with results element; Added transition to results display; Begin adding # results found / no results messaging; Moved class .off to _globals.scss;

open #250 open #260 vista#132 Major refactoring to markup and CSS to apply flex layout strategy to object headers; Flex necessary to fix 'overflow: hidden' setting applied in open90 to object-browse-bar that was preventing the view switcher menu from appearing; Browse, edit, mobile, frames in layouts, etc. all visually checked in a first go round; Mobile classes tweaked to fix left and right margin problems resulting from open90 changes; z-indexing of Inspector pane and splitters changed to allow primary pane elements to overflow beneath when primary pane width is severely constrained;

open #250 Snow theme constants missing were causing the Snow theme CSS to not compile - this commit restores those values; TO-DO: values defined are a bit out of sync between Espresso and Snow and need to be realigned;

open #250 Fixed min and max widths for panes to prevent weird pane expand/collapse behaviors, like Inspector covering the main view area; Added min-width to .holder-all element;

open #250 Moved new min/max definitions into desktop-only portion of layout.scss to not affect mobile layout;

open #250

open #250 Removed un-needed markup around treeview includes and mct-reps; moved CSS class defs into mct-include and mct-rep for search and tree respectively;

open #250

open #250 Changed flex-elem to use margin-bottom instead of top; Added margin-top to .search-results element;

open #250 open #157 Create button now hidden by default in mobile via usage of CSS; can be re-displayed by including 'example/mobile' bundle;

open #250 open #157 Fixed CSS selector that wasn't properly targeting the Create menu in a mobile context;

open #250

VWoeltjen · 2015-11-10T22:25:01Z

platform/commonUI/browse/res/templates/browse/object-header.html

-        <mct-representation key="'menu-arrow'" mct-object='domainObject'></mct-representation>
-    </span>
-</div>
+<!--<div class='object-header'> Moved up to browse-object.html and frame.html. TO-DO: make sure this is Ok!-->


Has this TODO been addressed?

Yes, it has. Will remove to-do.

charlesh88 · 2015-11-10T22:32:08Z

Uh, if you start looking for commented dead code in my sass, you're going to continually trip over it. This hasn't come up as a general issue previously, although Pete called me out on it in one particular instance a little while ago.

Let me go through and kill the comments in the sass in this branch, I'll push up a new version shortly. I'd really like the sass in #250 for something I'm doing in #279, so I'm highly motivated to get this integrated.

VWoeltjen · 2015-11-10T22:36:24Z

Thanks - I'm trying to be more diligent about catching little things like that during code review (it's the only time they really get caught). Reassign to me when you're ready; I'll keep an eye out to complete the merge.

charlesh88 · 2015-11-10T22:37:53Z

No prob, and thanks. Moving forward I'll ensure that commented stuff is stamped out before doing pull reqs.

VWoeltjen · 2015-11-10T22:40:48Z

Hm, so running this branch (open250) locally, I get a strange display (left pane starts collapsed):

If I manually adjust the splitter next to the tree, the tree displays correctly, but I still have no inspector.

Wonder if this is a more severe case of #169? (I'm viewing on my little laptop screen)

charlesh88 · 2015-11-10T22:54:33Z

Re. "...I get a strange display...": was totally able to duplicate this in the Chrome mobile emulator: this is #169. I'm going to make fixing that a priority so you (and others with small laptops) won't be impacted by this.

open #250 open #274 scss, html scoured for commented dead code, leftover TO-DOs and other unnecessary comments;

charlesh88 · 2015-11-10T23:13:26Z

Ok, just pushed an updated version of open250 - should be completely cleaned up.

Review and integrate open250

VWoeltjen · 2015-11-10T23:20:08Z

Reviewer Checklist

Changes appear to address issue? Y
Appropriate unit tests included? N/A *
Code style and in-line documentation are appropriate? Y
Commit messages meet standards? Y

* changes to CSS & templates only, out of scope for automated testing

charlesh88 · 2015-11-10T23:55:35Z

Awesome, thanks!

charlesh88 and others added 15 commits November 5, 2015 10:11

[Frontend] Convert main left pane elements to flex layout

8ca22cc

open #250 Markup mods in browse.html; Renamed .holder-create-and-search to .holder-treeview-elements; TO-DO: fix search results to scroll properly;

[Frontend] Convert main left pane elements to flex layout

c0ac3a0

open #250 Significant changes to flex classes, markup in browse.html and search.html to support better flex layout; Search results now scrolls properly; Significant refactoring and cleanups in search and _search.scss;

[Frontend] Cleanups and enhancements to search

063e97f

open #250 Fixed margin/spacing problems with results element; Added transition to results display; Begin adding # results found / no results messaging; Moved class .off to _globals.scss;

Merge remote-tracking branch 'github/master' into open250

76e15f2

[Frontend] Restoring dropped Snow theme constants

fd135a1

open #250 Snow theme constants missing were causing the Snow theme CSS to not compile - this commit restores those values; TO-DO: values defined are a bit out of sync between Espresso and Snow and need to be realigned;

[Frontend] Min/max widths on panes adjusted

2b92c16

open #250 Fixed min and max widths for panes to prevent weird pane expand/collapse behaviors, like Inspector covering the main view area; Added min-width to .holder-all element;

[Frontend] Min/max widths on panes adjusted

a5fa117

open #250 Moved new min/max definitions into desktop-only portion of layout.scss to not affect mobile layout;

[Frontend] Fix to z-indexing for search "types" menu

55d3a27

open #250

[Frontend] Consolidated treeview elements markup

2a201df

open #250 Removed un-needed markup around treeview includes and mct-reps; moved CSS class defs into mct-include and mct-rep for search and tree respectively;

[Frontend] Fixed padding in search results

c0dc30e

open #250

[Frontend] Adjusted margin strategy in left pane elements

cfc9b61

open #250 Changed flex-elem to use margin-bottom instead of top; Added margin-top to .search-results element;

[Frontend] Hide/show Create button in mobile

d8ced8f

open #250 open #157 Create button now hidden by default in mobile via usage of CSS; can be re-displayed by including 'example/mobile' bundle;

[Frontend] Fixed Create menu in mobile

e1110a2

open #250 open #157 Fixed CSS selector that wasn't properly targeting the Create menu in a mobile context;

[Merge] Safety merge-in of latest master

72e0304

open #250

charlesh88 assigned VWoeltjen Nov 10, 2015

charlesh88 added this to the Banks milestone Nov 10, 2015

This was referenced Nov 10, 2015

[Mobile] Suppress Create button in Mobile #157

Closed

Tree does not properly display when search bundle is excluded #185

Closed

Convert left pane to use flexbox layout #250

Closed

View switcher menu not appearing #260

Closed

VWoeltjen reviewed Nov 10, 2015
View reviewed changes

VWoeltjen assigned charlesh88 and unassigned VWoeltjen Nov 10, 2015

[Code style] Cleaned out comments

d6d95fe

open #250 open #274 scss, html scoured for commented dead code, leftover TO-DOs and other unnecessary comments;

VWoeltjen added a commit that referenced this pull request Nov 10, 2015

Merge pull request #274 from nasa/open250

7264a71

Review and integrate open250

VWoeltjen merged commit 7264a71 into master Nov 10, 2015

larkin deleted the open250 branch May 3, 2016 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Review and integrate open250 #274

Review and integrate open250 #274

charlesh88 commented Nov 10, 2015

VWoeltjen Nov 10, 2015

charlesh88 Nov 10, 2015

charlesh88 commented Nov 10, 2015

VWoeltjen commented Nov 10, 2015

charlesh88 commented Nov 10, 2015

VWoeltjen commented Nov 10, 2015

charlesh88 commented Nov 10, 2015

charlesh88 commented Nov 10, 2015

VWoeltjen commented Nov 10, 2015

charlesh88 commented Nov 10, 2015

Review and integrate open250 #274

Review and integrate open250 #274

Conversation

charlesh88 commented Nov 10, 2015

VWoeltjen Nov 10, 2015

Choose a reason for hiding this comment

charlesh88 Nov 10, 2015

Choose a reason for hiding this comment

charlesh88 commented Nov 10, 2015

VWoeltjen commented Nov 10, 2015

charlesh88 commented Nov 10, 2015

VWoeltjen commented Nov 10, 2015

charlesh88 commented Nov 10, 2015

charlesh88 commented Nov 10, 2015

VWoeltjen commented Nov 10, 2015

Reviewer Checklist

charlesh88 commented Nov 10, 2015