Tradeshift / tradeshift-ui

The Tradeshift UI Library & Framework
https://ui.tradeshift.com
Other
33 stars 45 forks source link

Make it configurable whether to add hamburger icon to topbar #99

Open sorenlouv opened 7 years ago

sorenlouv commented 7 years ago

Currently the hamburger icon will automatically be added to an app's topbar, if it is deemed to be in mobile view. However, certain apps are always displayed inside a fixed viewport (of small size), and will therefore always be "in mobile view" even on desktop.

Is it possible to make it configurable, whether to add the hamburger icon to the topbar?

screen shot 2017-01-03 at 2 00 15 pm

wiredearp commented 7 years ago

Right. To provide a counter argument, then, the TopBar is (conceptually) a place for top level navigation (via the tabs) and top level actions (via the buttons) that affect the "whole page". If an app is now embedded somewhere on the page, even if technically it inside an iframe, should it then have a TopBar? If all the actions (via the buttons) are local to the app, then perhaps it should really be a ToolBar? These are the (conceptual) rules:

  1. There can be only one TopBar on the page, it is "global".
  2. There can however be multiple ToolBars on the page.

I think that this particular panel is a cornercase scenario because it's not really a part of the app even if it is a part of the page, so we can either:

  1. Support no-hamburger-icon on the TopBar for whatever reason the developer prefer - or
  2. Explain clearly in the documentation that only the main app can have a TopBar and that all embedded apps must use a ToolBar (at least for actions that are local the the app, they can otherwise inject buttons into the one-and-only TopBar if they like).

One can imagine that the Collaboration Panel was moved to the bottom of the screen, or to a box in the middle of the screen, just as an exercise to illustrate the potential disruption of using a TopBar outside of the "main app". Let's at least discuss before we proceed cc @jerf and @DocGroth.

jerf commented 7 years ago

@wiredearp @sqren Moth's argument makes sense especially as the layout we want to eventually achieve for the Universal Inbox would not have a header (TopBar) and be inside the chrome.

wiredearp commented 7 years ago

In that case, @sqren, we should probably rig the Collaboration up with manualLayout: true in the manifest.json so that V4 doesn't render the TopBar; and then hardcode the basic layout as seen in http://ui.tradeshift.com/#getstarted/layout/ (minus the TopBar). You can then create a ToolBar with an API that is equivalent to the TopBar:

ts.ui.ToolBar.buttons([
    {
        icon: 'ts-icon-close',
        onclick: function() {}
    }
]);

We'll make sure to copy the methods dark() and green() and so on onto the ToolBar before the next release so that you can make it black if you like. We'll also create a special CSS classname to render the buttons without a border since that particular ts-icon-close button traditionally doesn't have one.

sorenlouv commented 7 years ago

The manualLayout: true sounds like a good solution. Will look into that.

jerf commented 7 years ago

@sqren: I'm OK with the user having to close the inbox to access the main menu in the mobile view until we get the new chrome.