You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by millayr <gi...@git.apache.org> on 2017/03/23 16:14:31 UTC

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

GitHub user millayr opened a pull request:

    https://github.com/apache/couchdb-fauxton/pull/883

    Navbar refactor

    Trying to take over https://github.com/apache/couchdb-fauxton/pull/826

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/millayr/couchdb-fauxton sidebar-refactor-recoloring

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/couchdb-fauxton/pull/883.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #883
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r109368776
  
    --- Diff: settings.json.default.json ---
    @@ -47,7 +51,9 @@
             "dest": "dist/debug/index.html",
             "variables": {
               "title": "Project Fauxton",
    -          "generationLabel": "Fauxton Couchapp"
    +          "generationLabel": "Fauxton Couchapp",
    --- End diff --
    
    This is so awesome.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108350409
  
    --- Diff: app/addons/fauxton/assets/less/navigation.less ---
    @@ -0,0 +1,177 @@
    +.faux-navbar {
    +  margin-top: 64px;
    +  background-color: @brandDark2;
    +
    +  position: absolute;
    +  left: 0;
    +  top: 0;
    +  bottom: 0;
    +  z-index: 5;
    +
    +}
    +
    +.faux-navbar__itemarea {
    +  .box-sizing(border-box);
    +  border-bottom: 1px solid @brandDark2;
    +  height: 48px;
    +  padding: 10px 20px;
    +  line-height: 24px;
    +}
    +
    +.faux-navbar__version-footer {
    +  color: @buttonText;
    +  font-size: 10px;
    +  text-align: center;
    +  background-color: @brandDark2;
    +}
    +
    +.faux-navbar__burger {
    +  background-color: @brandDark2;
    +  padding: 19px 0 18px 18px;
    +  position: fixed;
    +  z-index: 100;
    +  top: 0;
    +}
    +
    +.faux-navbar--narrow {
    +  width: @collapsedNavWidth;
    +}
    +
    +.faux-navbar--wide {
    +  width: @navWidth;
    +}
    +
    +.faux-navbar__burger:hover .faux-navbar__burger__icon {
    +  color: @navIconActive;
    +}
    +
    +.faux-navbar__burger__icon {
    +  color: @navIconColor;
    +  font-size: 27px;
    +}
    +
    +.faux-navbar__burger__icon--flipped:before{
    +  -moz-transform: scale(-1, 1);
    +  -webkit-transform: scale(-1, 1);
    +  -o-transform: scale(-1, 1);
    +  -ms-transform: scale(-1, 1);
    +  transform: scale(-1, 1);
    +}
    +
    +.faux-navbar__link, .faux-logout__link, .faux-login__link {
    +  display: block;
    +  text-decoration: none;
    +  cursor: pointer;
    +}
    +
    +.faux-navbar__link--active {
    +  background-color: @brandHighlight;
    +  text-decoration: none;
    +}
    +
    +.faux-navbar__link--inactive {
    +  background-color: @brandDark1;
    +}
    +
    +.faux-navbar__link:hover, .faux-logout__link:hover, .faux-login__link:hover {
    +  background-color: @hoverHighlight;
    +  text-decoration: none;
    +}
    +
    +.faux-navbar__link:active {
    +  text-decoration: none;
    +}
    +
    +.faux-navbar__link:hover .faux-navbar__icon:before {
    +  color: @navIconActive;
    +}
    +
    +.faux-navbar__link--active .faux-navbar__icon:before {
    +  color: @navIconActive;
    +}
    +
    +.faux-navbar__icon {
    +  margin-right: 14px;
    +  color: @navIconColor;
    +  font-size: 24px;
    +  vertical-align: middle;
    +}
    +
    +.faux-navbar__text {
    +  margin: 0;
    +  color: @buttonText;
    +  vertical-align: middle;
    +  font-size: 16px;
    +  font-weight: normal;
    +  font-family: Helvetica,sans-serif;
    +  font-weight: 400;
    +}
    +
    +.faux-navbar__logout__text {
    +  font-size: 12px;
    +  color: @buttonText;
    +}
    +
    +.faux-navbar__logout__textcontainer {
    +  text-align: center;
    +  color: @buttonText;
    +}
    +
    +.faux-navbar__logout__textcontainer--narrow {
    +  padding-bottom: 4px;
    +  padding: 20px 0;
    +}
    +
    +.faux-navbar__logout__textcontainer--wide {
    +  overflow: hidden;
    +  text-overflow: ellipsis;
    +  white-space: nowrap;
    +  padding: 20px;
    +}
    +
    +.faux-navbar__login__textcontainer {
    +  text-align: center;
    +  color: @buttonText;
    +}
    +
    +.faux-navbar__login__textcontainer--narrow {
    +  padding-bottom: 4px;
    +  padding: 20px 0;
    +}
    +
    +.faux-navbar__login__textcontainer--wide {
    +  padding: 20px;
    +}
    +
    +.faux-navbar__brand {
    +  margin: 20px 0 20px 0;
    +  height: 50px;
    +  padding: 10px 10px 10px 10px;
    +  float: none;
    +  background: @brandDark2;
    +}
    +
    +.faux-navbar__brand-logo {
    +  display: block;
    +  height: 100%;
    +  margin-top: 10px;
    +}
    +
    +.faux-navbar__brand-logo--wide {
    +  background: url(../../../../../assets/img/CouchDB-negative-logo.png) no-repeat 23px 0px;
    --- End diff --
    
    It would be nice to set these logo's via our settings.json file. I know the pouchdb team would like to override these


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r109031856
  
    --- Diff: app/addons/fauxton/navigation/stores.js ---
    @@ -94,8 +107,7 @@ Stores.NavBarStore = FauxtonAPI.Store.extend({
       },
     
       toggleMenu () {
    -    app.utils.localStorageSet(FauxtonAPI.constants.LOCAL_STORAGE.SIDEBAR_MINIMIZED,
    -                              !this.isMinimized());
    +    this._isMinimized = !this._isMinimized;
    --- End diff --
    
    I think I'd prefer to leave this as-is.  The new logic forces the sidebar to be minimized by default, showing users that they have much more screen real estate to work with, which is inline with the user research and feedback.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108353902
  
    --- Diff: app/addons/fauxton/navigation/components/NavBar.js ---
    @@ -0,0 +1,107 @@
    +// Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +// use this file except in compliance with the License. You may obtain a copy of
    +// the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing, software
    +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +// License for the specific language governing permissions and limitations under
    +// the License.
    +
    +import React, { Component } from 'react';
    +
    +import Footer from './Footer';
    +import Burger from './Burger';
    +import NavLink from './NavLink';
    +import Brand from './Brand';
    +import LogoutButton from './LogoutButton';
    +import LoginButton from './LoginButton';
    +
    +import Actions from "../actions";
    +
    +import classNames from 'classnames';
    +import _ from 'lodash';
    +
    +class NavBar extends Component {
    +
    +  createLinks (links) {
    +    const { activeLink, isMinimized } = this.props;
    +
    +    return _.map(links, (link, i) => {
    --- End diff --
    
    Can you rather do `links.map`. We don't need to use lodash for that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by justin-mcdavid-ibm <gi...@git.apache.org>.
Github user justin-mcdavid-ibm commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108506622
  
    --- Diff: app/addons/fauxton/navigation/stores.js ---
    @@ -94,8 +107,7 @@ Stores.NavBarStore = FauxtonAPI.Store.extend({
       },
     
       toggleMenu () {
    -    app.utils.localStorageSet(FauxtonAPI.constants.LOCAL_STORAGE.SIDEBAR_MINIMIZED,
    -                              !this.isMinimized());
    +    this._isMinimized = !this._isMinimized;
    --- End diff --
    
    We made the decision to open the dashboard with main-nav collapsed, and a clearer trigger for expanding it back out.  I don't think preferences need to be maintained across sessions, especially if the default state is the one providing more screen real estate.
    
    These decisions came out of user feedback regarding confusion on the "hamburger" icon;s functionality, and requests for more visual space to work with.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108349952
  
    --- Diff: app/addons/fauxton/appwrapper.js ---
    @@ -49,27 +53,52 @@ class ContentWrapper extends React.Component {
       }
     }
     
    -const App = ({router}) => {
    -  return (
    -    <div>
    -      <PermanentNotification />
    -      <div id="notifications">
    -        <NotificationController />
    -      </div>
    -      <div role="main" id="main">
    -        <div id="app-container">
    -          <div className="wrapper">
    -            <div className="pusher">
    -              <ContentWrapper router={router} />
    -            </div>
    -            <div id="primary-navbar">
    -              <NavBar/>
    +class App extends React.Component {
    +  constructor (props) {
    +    super(props);
    +    this.state = this.getStoreState();
    +  }
    +
    +  getStoreState () {
    +    return {
    +      isPrimaryNavMinimized: navBarStore.isMinimized()
    +    };
    +  }
    +
    +  componentDidMount () {
    +    navBarStore.on('change', this.onChange, this);
    +  }
    +
    +  onChange () {
    +    this.setState(this.getStoreState());
    +  }
    +
    +  render () {
    +    const mainClass = classNames(
    --- End diff --
    
    Nice, this is very cool idea


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108499644
  
    --- Diff: app/addons/fauxton/navigation/stores.js ---
    @@ -10,24 +10,37 @@
     // License for the specific language governing permissions and limitations under
     // the License.
     
    -import app from "../../../app";
     import FauxtonAPI from "../../../core/api";
     import ActionTypes from "./actiontypes";
    -const Stores = {};
     
    +import _ from 'lodash';
    --- End diff --
    
    removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108354475
  
    --- Diff: app/addons/fauxton/navigation/stores.js ---
    @@ -10,24 +10,37 @@
     // License for the specific language governing permissions and limitations under
     // the License.
     
    -import app from "../../../app";
     import FauxtonAPI from "../../../core/api";
     import ActionTypes from "./actiontypes";
    -const Stores = {};
     
    +import _ from 'lodash';
    --- End diff --
    
    I don't think this is needed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108349694
  
    --- Diff: app/addons/auth/base.js ---
    @@ -19,58 +19,46 @@ Auth.session = new Auth.Session();
     FauxtonAPI.setSession(Auth.session);
     app.session = Auth.session;
     
    -Auth.initialize = function () {
     
    -  FauxtonAPI.addHeaderLink({
    -    id: 'auth',
    -    title: 'Login',
    -    href: '#/login',
    -    icon: 'fonticon-user',
    -    bottomNav: true
    -  });
    +function cleanupAuthSection () {
    +  FauxtonAPI.removeHeaderLink({ id: 'auth', bottomNav: true });
    +}
    +
    +Auth.initialize = function () {
     
       Auth.session.on('change', function () {
    -    var session = Auth.session;
    -    var link = {};
    +    const session = Auth.session;
     
         if (session.isAdminParty()) {
    -      link = {
    +      const link = {
             id: 'auth',
             title: 'Admin Party!',
             href: '#/createAdmin',
             icon: 'fonticon-user',
             bottomNav: true
           };
    +
    +      cleanupAuthSection();
    +      FauxtonAPI.addHeaderLink(link);
    +      FauxtonAPI.hideLogin();
    +
         } else if (session.isLoggedIn()) {
    -      link = {
    +      const link = {
    --- End diff --
    
    This doesn't seem  correct, the `link` variable is declared twice and then used outside its scope 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108354112
  
    --- Diff: app/addons/fauxton/navigation/container/NavBar.js ---
    @@ -0,0 +1,77 @@
    +// Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +// use this file except in compliance with the License. You may obtain a copy of
    +// the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing, software
    +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +// License for the specific language governing permissions and limitations under
    +// the License.
    +
    +import FauxtonAPI from "../../../../core/api";
    +import React from "react";
    +
    +import ReactDOM from "react-dom";
    +import Stores from "../stores";
    +
    +import NavBar from '../components/NavBar';
    +
    +const navBarStore = Stores.navBarStore;
    +
    +const NavBarContainer = React.createClass({
    +
    +  getStoreState () {
    +    return {
    +      navLinks: navBarStore.getNavLinks(),
    +      bottomNavLinks: navBarStore.getBottomNavLinks(),
    +      footerNavLinks: navBarStore.getFooterNavLinks(),
    +      activeLink: navBarStore.getActiveLink(),
    +      version: navBarStore.getVersion(),
    +      isMinimized: navBarStore.isMinimized(),
    +      isNavBarVisible: navBarStore.isNavBarVisible(),
    +
    +      isLoginSectionVisible: navBarStore.getIsLoginSectionVisible(),
    +      isLoginVisibleInsteadOfLogout: navBarStore.getIsLoginVisibleInsteadOfLogout()
    +    };
    +  },
    +
    +  getInitialState () {
    +    return this.getStoreState();
    +  },
    +
    +  onChange () {
    +    this.setState(this.getStoreState());
    +  },
    +
    +  setMenuState () {
    +    FauxtonAPI.Events.trigger(FauxtonAPI.constants.EVENTS.NAVBAR_SIZE_CHANGED, this.state.isMinimized);
    --- End diff --
    
    Is this trigger event still needed. Could we rather use a store instead. I would like to remove all our trigger events as much as possible.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton issue #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the issue:

    https://github.com/apache/couchdb-fauxton/pull/883
  
    This is coming along nicely. I'm seeing two issues. 
    
    ![https://www.dropbox.com/s/428id1nw7yw2svw/Screenshot%202017-04-03%2013.00.08.png?dl=1](https://www.dropbox.com/s/428id1nw7yw2svw/Screenshot%202017-04-03%2013.00.08.png?dl=1) My accounts/reset password block is hidden.
    
    Then if I make my screen narrow. The navbar struggles a bit:
    ![https://www.dropbox.com/s/apoj4o790xfuy3o/Screenshot%202017-04-03%2013.01.59.png?dl=1](https://www.dropbox.com/s/apoj4o790xfuy3o/Screenshot%202017-04-03%2013.01.59.png?dl=1)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108353139
  
    --- Diff: app/addons/fauxton/navigation/actiontypes.js ---
    @@ -19,5 +19,9 @@ export default {
       NAVBAR_SET_VERSION_INFO: 'NAVBAR_SET_VERSION_INFO',
       NAVBAR_ACTIVE_LINK: 'NAVBAR_ACTIVE_LINK',
       NAVBAR_HIDE: 'NAVBAR_HIDE',
    -  NAVBAR_SHOW: 'NAVBAR_SHOW'
    +  NAVBAR_SHOW: 'NAVBAR_SHOW',
    +
    --- End diff --
    
    Minor, but no space is needed here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108499467
  
    --- Diff: app/addons/fauxton/appwrapper.js ---
    @@ -49,27 +53,52 @@ class ContentWrapper extends React.Component {
       }
     }
     
    -const App = ({router}) => {
    -  return (
    -    <div>
    -      <PermanentNotification />
    -      <div id="notifications">
    -        <NotificationController />
    -      </div>
    -      <div role="main" id="main">
    -        <div id="app-container">
    -          <div className="wrapper">
    -            <div className="pusher">
    -              <ContentWrapper router={router} />
    -            </div>
    -            <div id="primary-navbar">
    -              <NavBar/>
    +class App extends React.Component {
    +  constructor (props) {
    +    super(props);
    +    this.state = this.getStoreState();
    +  }
    +
    +  getStoreState () {
    +    return {
    +      isPrimaryNavMinimized: navBarStore.isMinimized()
    +    };
    +  }
    +
    +  componentDidMount () {
    +    navBarStore.on('change', this.onChange, this);
    +  }
    +
    +  onChange () {
    +    this.setState(this.getStoreState());
    +  }
    +
    +  render () {
    +    const mainClass = classNames(
    --- End diff --
    
    thanks :D


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108354336
  
    --- Diff: app/addons/fauxton/navigation/stores.js ---
    @@ -94,8 +107,7 @@ Stores.NavBarStore = FauxtonAPI.Store.extend({
       },
     
       toggleMenu () {
    -    app.utils.localStorageSet(FauxtonAPI.constants.LOCAL_STORAGE.SIDEBAR_MINIMIZED,
    -                              !this.isMinimized());
    +    this._isMinimized = !this._isMinimized;
    --- End diff --
    
    This means we are no longer keeping state of what the user preferred in terms of minimized or open navbar?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108499571
  
    --- Diff: app/addons/fauxton/navigation/components/NavBar.js ---
    @@ -0,0 +1,107 @@
    +// Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +// use this file except in compliance with the License. You may obtain a copy of
    +// the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing, software
    +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +// License for the specific language governing permissions and limitations under
    +// the License.
    +
    +import React, { Component } from 'react';
    +
    +import Footer from './Footer';
    +import Burger from './Burger';
    +import NavLink from './NavLink';
    +import Brand from './Brand';
    +import LogoutButton from './LogoutButton';
    +import LoginButton from './LoginButton';
    +
    +import Actions from "../actions";
    +
    +import classNames from 'classnames';
    +import _ from 'lodash';
    +
    +class NavBar extends Component {
    +
    +  createLinks (links) {
    +    const { activeLink, isMinimized } = this.props;
    +
    +    return _.map(links, (link, i) => {
    --- End diff --
    
    done


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108499424
  
    --- Diff: app/addons/auth/base.js ---
    @@ -19,58 +19,46 @@ Auth.session = new Auth.Session();
     FauxtonAPI.setSession(Auth.session);
     app.session = Auth.session;
     
    -Auth.initialize = function () {
     
    -  FauxtonAPI.addHeaderLink({
    -    id: 'auth',
    -    title: 'Login',
    -    href: '#/login',
    -    icon: 'fonticon-user',
    -    bottomNav: true
    -  });
    +function cleanupAuthSection () {
    +  FauxtonAPI.removeHeaderLink({ id: 'auth', bottomNav: true });
    +}
    +
    +Auth.initialize = function () {
     
       Auth.session.on('change', function () {
    -    var session = Auth.session;
    -    var link = {};
    +    const session = Auth.session;
     
         if (session.isAdminParty()) {
    -      link = {
    +      const link = {
             id: 'auth',
             title: 'Admin Party!',
             href: '#/createAdmin',
             icon: 'fonticon-user',
             bottomNav: true
           };
    +
    +      cleanupAuthSection();
    +      FauxtonAPI.addHeaderLink(link);
    +      FauxtonAPI.hideLogin();
    +
         } else if (session.isLoggedIn()) {
    -      link = {
    +      const link = {
    --- End diff --
    
    fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108499614
  
    --- Diff: app/addons/fauxton/navigation/container/NavBar.js ---
    @@ -0,0 +1,77 @@
    +// Licensed under the Apache License, Version 2.0 (the "License"); you may not
    +// use this file except in compliance with the License. You may obtain a copy of
    +// the License at
    +//
    +//   http://www.apache.org/licenses/LICENSE-2.0
    +//
    +// Unless required by applicable law or agreed to in writing, software
    +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
    +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
    +// License for the specific language governing permissions and limitations under
    +// the License.
    +
    +import FauxtonAPI from "../../../../core/api";
    +import React from "react";
    +
    +import ReactDOM from "react-dom";
    +import Stores from "../stores";
    +
    +import NavBar from '../components/NavBar';
    +
    +const navBarStore = Stores.navBarStore;
    +
    +const NavBarContainer = React.createClass({
    +
    +  getStoreState () {
    +    return {
    +      navLinks: navBarStore.getNavLinks(),
    +      bottomNavLinks: navBarStore.getBottomNavLinks(),
    +      footerNavLinks: navBarStore.getFooterNavLinks(),
    +      activeLink: navBarStore.getActiveLink(),
    +      version: navBarStore.getVersion(),
    +      isMinimized: navBarStore.isMinimized(),
    +      isNavBarVisible: navBarStore.isNavBarVisible(),
    +
    +      isLoginSectionVisible: navBarStore.getIsLoginSectionVisible(),
    +      isLoginVisibleInsteadOfLogout: navBarStore.getIsLoginVisibleInsteadOfLogout()
    +    };
    +  },
    +
    +  getInitialState () {
    +    return this.getStoreState();
    +  },
    +
    +  onChange () {
    +    this.setState(this.getStoreState());
    +  },
    +
    +  setMenuState () {
    +    FauxtonAPI.Events.trigger(FauxtonAPI.constants.EVENTS.NAVBAR_SIZE_CHANGED, this.state.isMinimized);
    --- End diff --
    
    removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton issue #883: Navbar refactor

Posted by garrensmith <gi...@git.apache.org>.
Github user garrensmith commented on the issue:

    https://github.com/apache/couchdb-fauxton/pull/883
  
    I'm happy with that
    
    All misspelling thanks to my iPhone.
    ________________________________
    From: Ryan Millay <no...@github.com>
    Sent: Thursday, March 30, 2017 10:41:00 PM
    To: apache/couchdb-fauxton
    Cc: garren smith; Comment
    Subject: Re: [apache/couchdb-fauxton] Navbar refactor (#883)
    
    
    @millayr commented on this pull request.
    
    ________________________________
    
    In app/addons/fauxton/navigation/stores.js<https://github.com/apache/couchdb-fauxton/pull/883#discussion_r109031856>:
    
    > @@ -94,8 +107,7 @@ Stores.NavBarStore = FauxtonAPI.Store.extend({
       },
    
       toggleMenu () {
    -    app.utils.localStorageSet(FauxtonAPI.constants.LOCAL_STORAGE.SIDEBAR_MINIMIZED,
    -                              !this.isMinimized());
    +    this._isMinimized = !this._isMinimized;
    
    
    I think I'd prefer to leave this as-is. The new logic forces the sidebar to be minimized by default, showing users that they have much more screen real estate to work with, which is inline with the user research and feedback.
    
    \u2014
    You are receiving this because you commented.
    Reply to this email directly, view it on GitHub<https://github.com/apache/couchdb-fauxton/pull/883#discussion_r109031856>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AAK9AkXB8ALnpNUsstRKQbsywKmcNWZHks5rrBNcgaJpZM4Mm_i->.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108499514
  
    --- Diff: app/addons/fauxton/navigation/actiontypes.js ---
    @@ -19,5 +19,9 @@ export default {
       NAVBAR_SET_VERSION_INFO: 'NAVBAR_SET_VERSION_INFO',
       NAVBAR_ACTIVE_LINK: 'NAVBAR_ACTIVE_LINK',
       NAVBAR_HIDE: 'NAVBAR_HIDE',
    -  NAVBAR_SHOW: 'NAVBAR_SHOW'
    +  NAVBAR_SHOW: 'NAVBAR_SHOW',
    +
    --- End diff --
    
    fixed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] couchdb-fauxton pull request #883: Navbar refactor

Posted by millayr <gi...@git.apache.org>.
Github user millayr commented on a diff in the pull request:

    https://github.com/apache/couchdb-fauxton/pull/883#discussion_r108500571
  
    --- Diff: app/addons/fauxton/navigation/stores.js ---
    @@ -94,8 +107,7 @@ Stores.NavBarStore = FauxtonAPI.Store.extend({
       },
     
       toggleMenu () {
    -    app.utils.localStorageSet(FauxtonAPI.constants.LOCAL_STORAGE.SIDEBAR_MINIMIZED,
    -                              !this.isMinimized());
    +    this._isMinimized = !this._isMinimized;
    --- End diff --
    
    Looks that way, yeah.  @justin-mcdavid-ibm: did you discuss this with @robertkowalski during the initial design phase for this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---