Make DesignDocSelector into a dumb component #650
Conversation
benkeen
force-pushed
the
design-doc-selector-update
branch
2 times, most recently
from
8155e8c to
6e7d807
Compare
|
Hey @robertkowalski, there's a failing test on this one, but it may have uncovered a related problem so I wanted to confirm the behaviour before fixing it. Here's the scenario (do this on master):
So far, this seems correct to me.
Is this behaviour correct? Seems strange to me... shouldn't that View now always just show the single document when you've selected _sum for a View, like the test is confirming? |
yes that sounds like a bug |
|
Great, thanks Robert. I'll patch that up with this ticket and write a test for it. |
benkeen
force-pushed
the
design-doc-selector-update
branch
from
6e7d807 to
de6e249
Compare
|
Okay, I pared back this ticket a bit. I was a little overambitious in my refactoring. The next PR [https://github.com//pull/651] moves the Views to their own page and will remove the need for some of the logic to dynamically reload the result list. |
benkeen
force-pushed
the
design-doc-selector-update
branch
from
de6e249 to
525a8ec
Compare
| return; | ||
| } | ||
|
|
||
| var doc = this.refs.designDocSelector.getDesignDoc(); |
There was a problem hiding this comment.
hmmm, not feeling comfortable by calling a getter on a view to get data. we used to do that with backbone a lot and it got us really into trouble.
maybe we can put that into a store and let the component fire an action? the dumb component does not need to be connected directly to the store, it could get the data updates as props from the parent component
There was a problem hiding this comment.
Blerg. I agree. Let me see what I can do.
benkeen
force-pushed
the
design-doc-selector-update
branch
from
525a8ec to
7331351
Compare
|
Here we go. I slimmed down the component further to remove the getter and the Backbone props (Document + design doc collection). |
|
+1 after using this.state |
| database: this.state.database, | ||
| newView: this.state.isNewView, | ||
| viewName: this.state.viewName, | ||
| designDoc: indexEditorStore.getSaveDesignDoc(), |
There was a problem hiding this comment.
this.state.saveDesignDoc
saves performance & is in general nicer :)
|
Nice, thanks Robert! Yeah I stuck the store call at that location because technically it was only needed at that point: and calling it always would call it before the data was actually ready. Not a big deal at all, I can just add in a clause to clear it up, and I agree it's cleaner to put them all in state. |
The dumbifies the DesignDocSelector to remove all ties a to store and passes everything via props so we can get more use out of it. Specifically, I need this for my next PR which includes a new, generic Clone Index modal that'll use this component in different contexts (i.e. all index types). - Includes validation in the component to make it more self-contained and allows for better UX by focusing on error fields. - Save action significantly simplied to remove custom update logic and always redirect to appropriate View page.
benkeen
force-pushed
the
design-doc-selector-update
branch
from
7331351 to
37cfb74
Compare
The dumbifies the DesignDocSelector to remove all ties a to
store and passes everything via props so we can get more use out
of it. Specifically, I need this for my next PR which includes
a new, generic Clone Index modal that'll use this component
in different contexts (i.e. all index types).
Also includes validation in the component to make it more
self-contained and allows for better UX by focusing on error fields.