Skip to content

Conversation

fredtruman
Copy link
Contributor

@fredtruman fredtruman commented Jan 16, 2017

These changes are intended to do three things:

  1. Change the overall sidebar / content from a fixed and absolute positioned layout to a flexbox layout in order to easily allow for a collapsible sidebar.
  2. Implement a simple collapsible sidebar.
  3. Implement a consistent set of markup and classes on all content views to add clean(er) styles for flexbox support. This means sticky header controls and the content that internally scrolls beneath.

Affects basically every view in Compass.

Expanded sidebar:
screenshot 2017-01-16 15 09 40

Collapsed sidebar:
screenshot 2017-01-16 15 09 45

Example here of flexbox support for content view in the documents view with variable height "controls":
screenshot 2017-01-16 15 10 22
screenshot 2017-01-16 15 11 41

return (
<div className="index-container header-margin">
<div className="index-container">
{/* NOT SURE if we need to wrap the controls-container in a readonly conditional as well. */}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was tricky, but I needed to break the render up so that the "controls-container" is a sibling to the "column-container", not a child.

Not sure who has the most context for this index view? @KeyboardTsundoku

Choose a reason for hiding this comment

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

I should know enough to help with this :)

but it seems to be working fine

{this.CollectionStore.isReadonly() ? this.renderReadonly() : this.renderComponent()}
</div>
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this kosher @KeyboardTsundoku ? Needed to move the controls out of the "column-container" and wrap them as a sibling in "controls-container"

Choose a reason for hiding this comment

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

should be fine, i'll do some testing with read-only views and see if there are any issues

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@samweaver-zz
Copy link

This is insanely awesome.
One minor thing I noticed - if you are at the "home" level and you go to the Performance tab and then collapse the side bar, it drops you back to the Databases tab.

Copy link

@satyasinha satyasinha left a comment

Choose a reason for hiding this comment

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

style-wise LGTM

but functional tests are broken and switching tabs at instance level is not working as @samweaver pointed out.

I'll have a look at the functional tests later + other changes 👍

@fredtruman
Copy link
Contributor Author

Ah, that's funny on RTSS view 🤔

@satyasinha
Copy link

I've made a separate PR for rebasing this branch with master: #749

it was getting a bit gnarly

@rueckstiess
Copy link
Contributor

rueckstiess commented Jan 18, 2017

A couple of comments after first using this new version:

  • the top collapse/expand bar is a little hard to hit, because it's so thin. How about making it same height as the icons in the collapsed state?
  • The top collapse/expand bar uses another grey tone. Lots of different grey tones going on in the sidebar. Maybe we can consolidate? It could use the same grey as the filter input.
  • the collapsed state gives me the impression that I can still click on / access these icons in the sidebar. The home button works, the reload works but it's useless because you don't see the databases, and the filter icon doesn't do anything. Suggestion to address all issues: As soon as the user clicks on one of the icons, the sidebar expands again. With the filtering, it would be nice if the focus was also set on the filter input immediately.
  • The empty space in the collapsed state almost screams for more shortcut icons. We could add another "database" icon (the one we're already using), that brings you to the corresponding database level, just like the "home" brings you to the instance level.

Otherwise it's fantastic and I'm glad we finally use flexbox everywhere and fixed the issue with variable heights of the control-containers. Much cleaner and more flexible this way. One can just add another "status/action bar" component without change style.

@fredtruman
Copy link
Contributor Author

Good points 👍

Will move comments to https://jira.mongodb.org/browse/COMPASS-683 and address in coming sprint.

@satyasinha satyasinha closed this Jan 20, 2017
@durran durran deleted the COMPASS-616 branch March 4, 2018 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants