couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From benkeen <...@git.apache.org>
Subject [GitHub] couchdb-fauxton pull request: Changes page Filters tab moved to Re...
Date Mon, 16 Feb 2015 22:09:10 GMT
Github user benkeen commented on the pull request:

    https://github.com/apache/couchdb-fauxton/pull/265#issuecomment-74579976
  
    Hey @garrensmith, I confess I was getting a bit confused about some things so I thought
I'd just experiment a bit and organize things in what I regard as a logical manner. We can
just revert it if I'm barking up the wrong tree.
    
    Structure (pretty much the same):
    
    ```xml
    <ChangesHeader />
    	<ChangesHeaderTab />
    	<ChangesFilter />
    		<Filter /> (list of N components)
    		<AddFilterForm />
    			<AddFilterFormContent />
    				<FilterTooltip />
    ```
    
    Testing: these would be the key components, like before:
    `<ChangesHeaderTab />` - test the hide/show of the tab
    `<ChangesFilter />` - test the add/remove filters, tooltip content
    
    Comments:
    
    - It seemed weird that the components were all nice and discrete but shared the same store,
so I split it into three: one for the header to track its hide/show state, one for the changes
filter to track the list of visible filters, and one for the AddFilterForm to track the state
of the text field. [Maybe a bit overengineered now?] I like the new arrangement because looking
at a component you can see by checking its store what data is related to it. Before, I was
losing track of what piece of state is associated with which component.
    - I added a fauxton/mixins.js file to cut down on boilerplate code - namely the this.store.on('change',
callback) assignment and destruction.
    - I moved as much code relating to a particular component to the component itself, and
stopped passing stuff down the chain. e.g. `<Filter />` now handles its own onRemove
event and just fires an action. I'm not sure this is in keeping with the "dumb component"
idea, but I find this way there's less dependencies between components and code is where I
expect to find it.
    - I'm still hazy on the need to separate `AddFilterForm` and the form content (`AddFilterFormContent`).
They seem so functionally entwined, it feels a bit unnecessary - I'll have to get you to explain
that one to me again.



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

Mime
View raw message