You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by robertkowalski <gi...@git.apache.org> on 2015/01/21 19:22:25 UTC

[GitHub] couchdb-fauxton pull request: Create Header-Control-Bar in React

GitHub user robertkowalski opened a pull request:

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

    Create Header-Control-Bar in React

    adds a bar with controls for interaction with documents

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

    $ git pull https://github.com/robertkowalski/couchdb-fauxton head-new-control

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

    https://github.com/apache/couchdb-fauxton/pull/242.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 #242
    
----
commit d92f759960bbae0014b1f2b24fb9ec827ee05f3b
Author: Robert Kowalski <ro...@apache.org>
Date:   2015-01-19T12:58:51Z

    Create Header-Control-Bar in React
    
    adds a bar with controls for interaction with documents

----


---
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: Create Header-Control-Bar in React

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/242#discussion_r23378339
  
    --- Diff: app/addons/documents/header/header.actions.js ---
    @@ -0,0 +1,62 @@
    +// 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.
    +
    +define([
    +  'app',
    +  'api',
    +  'addons/documents/header/header.actiontypes'
    +],
    +function (app, FauxtonAPI, ActionTypes) {
    +
    +  return {
    +    toggleCollapseDocuments: function () {
    +      FauxtonAPI.dispatch({
    +        type: ActionTypes.COLLAPSE_DOCUMENTS
    +      });
    +
    +      FauxtonAPI.Events.trigger('headerbar:collapse');
    --- End diff --
    
    Can you make all of the events constants.


---
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: Create Header-Control-Bar in React

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/242#issuecomment-71023491
  
    @seanbarclay fading both header leads to space issues on small screens, so i decided to just fade the alternative header for now. i created a video that shows the issue: https://cloudup.com/c9kWmUm7_Km - the other header is pushed down as both elements are fading but are taking space.
    
    we could throw a ton of code on the problem but it would make the css and code quite complex.
    
    what do you think?


---
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: Create Header-Control-Bar in React

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/242#issuecomment-71878757
  
    merged as f5f7f4b93a2ac9f63b21d9a0a5359285e6f2da38 and 25dab477415d417019cf863db393470b11e5d350


---
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: Create Header-Control-Bar in React

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/242#discussion_r23518669
  
    --- Diff: app/addons/documents/header/header.actions.js ---
    @@ -0,0 +1,62 @@
    +// 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.
    +
    +define([
    +  'app',
    +  'api',
    +  'addons/documents/header/header.actiontypes'
    +],
    +function (app, FauxtonAPI, ActionTypes) {
    +
    +  return {
    +    toggleCollapseDocuments: function () {
    +      FauxtonAPI.dispatch({
    +        type: ActionTypes.COLLAPSE_DOCUMENTS
    +      });
    +
    +      FauxtonAPI.Events.trigger('headerbar:collapse');
    --- End diff --
    
    ok cool that makes sense.


---
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: Create Header-Control-Bar in React

Posted by seanbarclay <gi...@git.apache.org>.
Github user seanbarclay commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/242#issuecomment-71256102
  
    @robertkowalski really nice work touching this up. 
    
    The only thing I'm noticing is that if you have all docs deselected, then you click on one or more docs in the list, nothing happens if you hit deselect. 
    
    Same thing goes if you have all docs in the list selected, then you deselect one and hit select all. It doesn't re-select the one unselected doc. 
    
    Lastly, "Select All" and "Deselect All" indicator colors should only be turned on if All or None of the docs are selected. For instance, if all docs were selected, "Select All" could be indicated as active (red). However, if you deselect >=1 docs in the list, then it should no longer be a true statement and therefore "Select All" should not be active, it should be back to normal and ready to toggle back on if clicked. 
    
    Ping me if you need further help there. 



---
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: Create Header-Control-Bar in React

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

    https://github.com/apache/couchdb-fauxton/pull/242#discussion_r23465923
  
    --- Diff: app/addons/documents/header/header.actions.js ---
    @@ -0,0 +1,62 @@
    +// 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.
    +
    +define([
    +  'app',
    +  'api',
    +  'addons/documents/header/header.actiontypes'
    +],
    +function (app, FauxtonAPI, ActionTypes) {
    +
    +  return {
    +    toggleCollapseDocuments: function () {
    +      FauxtonAPI.dispatch({
    +        type: ActionTypes.COLLAPSE_DOCUMENTS
    +      });
    +
    +      FauxtonAPI.Events.trigger('headerbar:collapse');
    --- End diff --
    
    I asked Ben regarding that because I sometimes see that they are added to the constants file and sometimes not:
    
    "Yeah. So here's my rationale: events that are relevant script-wide (stuff in /addons/fauxton) for example should be there. Stuff that's addon-specific shouldn't."


---
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: Create Header-Control-Bar in React

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/242#discussion_r23518829
  
    --- Diff: app/addons/documents/header/header.react.jsx ---
    @@ -0,0 +1,239 @@
    +// 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.
    +
    +define([
    +  'app',
    +  'api',
    +  'react',
    +  'addons/documents/header/header.stores',
    +  'addons/documents/header/header.actions'
    +],
    +
    +function (app, FauxtonAPI, React, Stores, Actions) {
    +  var ReactCSSTransitionGroup = React.addons.CSSTransitionGroup;
    +  var headerBarStore = Stores.headerBarStore;
    +  var alternativeHeaderBarStore = Stores.alternativeHeaderBarStore;
    +
    +  // this will be a global component
    +  var ToggleHeaderButton = React.createClass({
    +    render: function () {
    +      var iconClasses = 'icon ' + this.props.fonticon + ' ' + this.props.innerClasses,
    +          containerClasses = 'button ' + this.props.containerClasses;
    +
    +      if (this.props.setEnabledClass) {
    +        containerClasses = containerClasses + ' js-headerbar-togglebutton-selected';
    +      }
    +
    +      return (
    +        <button
    +          title={this.props.title}
    +          disabled={this.props.disabled}
    +          onClick={this.props.toggleCallback}
    +          className={containerClasses}
    +          >
    +          <i className={iconClasses}></i><span>{this.props.text}</span>
    +        </button>
    +      );
    +    }
    +  });
    +
    +  var AlternateHeaderControlBarController = React.createClass({
    --- End diff --
    
    I think we should rename the `AlternativeHeader` to something like `BulkDoumentHeader`. Since when that header is displayed we editing the documents in bulk. Its a little more descriptive than just `AlternativeHeader`


---
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: Create Header-Control-Bar in React

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

    https://github.com/apache/couchdb-fauxton/pull/242#issuecomment-71859635
  
    This is looking good. Nice tests. +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: Create Header-Control-Bar in React

Posted by robertkowalski <gi...@git.apache.org>.
Github user robertkowalski closed the pull request at:

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


---
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: Create Header-Control-Bar in React

Posted by seanbarclay <gi...@git.apache.org>.
Github user seanbarclay commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/242#issuecomment-70980379
  
    Hey @robertkowalski - nice job. I'm super excited to see this plugged in. There's a few minor tweaks needed here:
    1) The padding between the select icon and copy should be 8px. It currently has none. 
    2) Make sure the padding on the select button group and the other select options have a padding that equals 20px on left and right. It's currently 17px (5px + 12px). Check out the screen shot of what spacing should look like when tweaked: https://www.dropbox.com/s/8j83rnqyjfrctqt/Screenshot%202015-01-21%2023.06.51.png?dl=0
    3) Font-size for icons should be 20px
    4) cancel button does not require a vertical rule line on the right
    5) add a quick (250 ms) fade in/out transition for menus.  You could also have the .alternative-header slide in from left to right and fade in. If you do the latter, add a background-color to the select button.
    
    Thanks!
    



---
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: Create Header-Control-Bar in React

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

    https://github.com/apache/couchdb-fauxton/pull/242#issuecomment-71433398
  
    @robertkowalski @seanbarclay do we need both Select All and Deselect All buttons displayed at all times? Wouldn't it make sense that initially the user only sees the Select All button. Then if they click on that then the Select All button disappears and the Deselect All button appears. Same for collapse and expand.


---
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: Create Header-Control-Bar in React

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

    https://github.com/apache/couchdb-fauxton/pull/242#issuecomment-71030009
  
    @robertkowalski this is coming along nicely. The code is looking excellent


---
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: Create Header-Control-Bar in React

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/242#discussion_r23686237
  
    --- Diff: app/addons/documents/header/header.react.jsx ---
    @@ -0,0 +1,245 @@
    +// 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.
    +
    +define([
    +  'app',
    +  'api',
    +  'react',
    +  'addons/documents/header/header.stores',
    +  'addons/documents/header/header.actions'
    +],
    +
    +function (app, FauxtonAPI, React, Stores, Actions) {
    +  var ReactCSSTransitionGroup = React.addons.CSSTransitionGroup;
    +  var headerBarStore = Stores.headerBarStore;
    +  var bulkDocumentHeaderStore = Stores.bulkDocumentHeaderStore;
    +
    +  // this will be a global component
    +  var ToggleHeaderButton = React.createClass({
    +    render: function () {
    +      var iconClasses = 'icon ' + this.props.fonticon + ' ' + this.props.innerClasses,
    +          containerClasses = 'button ' + this.props.containerClasses;
    +
    +      if (this.props.setEnabledClass) {
    +        containerClasses = containerClasses + ' js-headerbar-togglebutton-selected';
    +      }
    +
    +      return (
    +        <button
    +          title={this.props.title}
    +          disabled={this.props.disabled}
    +          onClick={this.props.toggleCallback}
    +          className={containerClasses}
    +          >
    +          <i className={iconClasses}></i><span>{this.props.text}</span>
    +        </button>
    +      );
    +    }
    +  });
    +
    +  var BulkDocumentHeaderController = React.createClass({
    +    getStoreState: function () {
    +      return {
    +        areDocumentsCollapsed: bulkDocumentHeaderStore.getCollapsedState(),
    +        isDeselectPossible: bulkDocumentHeaderStore.getIsDeselectPossible(),
    +        isSelectAllPossible: bulkDocumentHeaderStore.getIsSelectAllPossible()
    +      };
    +    },
    +
    +    getInitialState: function () {
    +      return this.getStoreState();
    +    },
    +
    +    componentDidMount: function () {
    +      bulkDocumentHeaderStore.on('change', this.onChange, this);
    +    },
    +
    +    componentWillUnmount: function() {
    +      bulkDocumentHeaderStore.off('change', this.onChange);
    +    },
    +
    +    onChange: function () {
    +      this.setState(this.getStoreState());
    +    },
    +
    +    render: function () {
    +      var baseClass = 'header-control-box header-control-square ',
    +          isDeselectPossible = this.state.isDeselectPossible,
    +          isSelectAllPossible = this.state.isSelectAllPossible,
    +          areDocumentsCollapsed = this.state.areDocumentsCollapsed;
    +
    +      return (
    +        <div className='alternative-header'>
    +          <ToggleHeaderButton
    +            fonticon={'fonticon-select-all'}
    +            toggleCallback={this.selectAllDocuments}
    +            innerClasses={''}
    +            containerClasses={baseClass + 'control-select-all'}
    +            text={''}
    +            setEnabledClass={!isSelectAllPossible}
    +            disabled={!isSelectAllPossible}
    +            title={'Select all Documents'} />
    +
    +          <ToggleHeaderButton
    +            fonticon={'fonticon-deselect-all'}
    +            toggleCallback={this.deSelectAllDocuments}
    +            innerClasses={''}
    +            containerClasses={baseClass + 'control-de-select-all'}
    +            text={''}
    +            setEnabledClass={!isDeselectPossible}
    +            disabled={!isDeselectPossible}
    +            title={'Deselect all Documents'} />
    +
    +          <ToggleHeaderButton
    +            fonticon={'fonticon-collapse'}
    +            toggleCallback={this.toggleCollapseDocuments}
    +            innerClasses={''}
    +            containerClasses={baseClass + 'control-collapse'}
    +            text={''}
    +            setEnabledClass={areDocumentsCollapsed}
    +            disabled={areDocumentsCollapsed}
    +            title={'Collapse all'} />
    +
    +          <ToggleHeaderButton
    +            fonticon={'fonticon-expand'}
    +            toggleCallback={this.toggleCollapseDocuments}
    +            innerClasses={''}
    +            containerClasses={baseClass + 'control-expand'}
    +            text={''}
    +            setEnabledClass={!areDocumentsCollapsed}
    +            disabled={!areDocumentsCollapsed}
    +            title={'Expand all'} />
    +
    +          <ToggleHeaderButton
    +            fonticon={'fonticon-trash'}
    +            toggleCallback={this.deleteSelected}
    +            innerClasses={''}
    +            containerClasses={baseClass + 'control-delete'}
    +            text={''}
    +            title={'Delete selected'} />
    +
    +          <ToggleHeaderButton
    +            fonticon={''}
    +            toggleCallback={this.cancelView}
    +            innerClasses={''}
    +            containerClasses={baseClass + 'control-cancel'}
    +            text={'Cancel'}
    +            title={'Switch to other view'} />
    +        </div>
    +      );
    +    },
    +
    +    toggleCollapseDocuments: function () {
    +      Actions.toggleCollapseDocuments();
    +    },
    +
    +    selectAllDocuments: function () {
    +      Actions.toggleSelectAllDocuments(false);
    +    },
    +
    +    deSelectAllDocuments: function () {
    +      Actions.toggleSelectAllDocuments(true);
    +    },
    +
    +    cancelView: function () {
    +      Actions.toggleHeaderControls();
    +    },
    +
    +    deleteSelected: function () {
    +      Actions.deleteSelected();
    +    }
    +  });
    +
    +  var HeaderBarController = React.createClass({
    +    getStoreState: function () {
    +      return {
    +        isToggled: headerBarStore.getStatus(),
    --- End diff --
    
    Pretty minor thing. But `getStatus` doesn't mean much. Maybe make it `getToggleStatus`. That explains it more.


---
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.
---