Closed
Bug 1094137
Opened 10 years ago
Closed 10 years ago
Create a common, shared Flux Store creator for Loop
Categories
(Hello (Loop) :: Client, defect)
Hello (Loop)
Client
Tracking
(firefox35 fixed, firefox36 fixed)
People
(Reporter: NiKo, Unassigned)
References
Details
(Whiteboard: [tech-debt])
Attachments
(1 file, 4 obsolete files)
72.82 KB,
patch
|
standard8
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
We need a common base Flux store creator, so we could avoid reimplementing & duplicating the store shared API by hand all the time. Example target usage: var StandaloneAppStore = loop.store.createStore({ registeredActions: [ "getWindowData" ], getWindowData: function(actionData) { // … } });
Assignee | ||
Comment 1•10 years ago
|
||
This patch brings a loop.store.createStore function to ease creating Flux storesi sharing a common stadard API. RoomStore and ActiveRoomStore have been migrated to use this new facility, other stores should follow in a possible part 2/follow up bug. Note that ConversationStore is harder to migrate because it extends Backbone.Model.
Attachment #8517699 -
Flags: review?(standard8)
Assignee | ||
Comment 2•10 years ago
|
||
Removed duplicate dispatcher passed as options to stores.
Attachment #8517699 -
Attachment is obsolete: true
Attachment #8517699 -
Flags: review?(standard8)
Attachment #8517703 -
Flags: review?(standard8)
Assignee | ||
Comment 3•10 years ago
|
||
Added BaseStore#dispatchAction to ease dispatching right from a store instance.
Attachment #8517703 -
Attachment is obsolete: true
Attachment #8517703 -
Flags: review?(standard8)
Attachment #8517952 -
Flags: review?(standard8)
Comment 4•10 years ago
|
||
Comment on attachment 8517952 [details] [diff] [review] Shared Flux store creator, v3. Review of attachment 8517952 [details] [diff] [review]: ----------------------------------------------------------------- This is good, I like the consistency it introduces and the functions it has. I suspect there's some small bitrot, and there's a few comments to be addressed, but I think now is a good time to land this. For the checkin comment, I'd go with something like "Bug 1094137 - Create a comment shared store creator for Loop." ::: browser/components/loop/content/shared/js/activeRoomStore.js @@ +19,5 @@ > * > * @extends {Backbone.Events} > * > * @param {Object} options - Options object > * @param {loop.Dispatcher} options.dispatch - The dispatcher for dispatching jsdoc needs updating - dispatcher is no longer part of options. ::: browser/components/loop/content/shared/js/roomStore.js @@ +54,5 @@ > * - {ActiveRoomStore} activeRoomStore An optional substore for active room > * state. > * > * @extends {Backbone.Events} > * @param {Object} options Options object. jsdoc needs updating - dispatcher is no longer part of options. ::: browser/components/loop/content/shared/js/store.js @@ +6,5 @@ > + > +var loop = loop || {}; > +loop.store = loop.store || {}; > + > +(function() { I kinda prefer this format: loop.store.createStore = (function() { as it gives you an idea of what's going to be returned for the function. @@ +35,5 @@ > + * Returns current store state. You can request a given state property by > + * providing the `key` argument. > + * > + * @param {String|undefined} key An optional state property name. > + * @return {Object} This might not be true if a key is specified - it could be string/number etc.
Attachment #8517952 -
Flags: review?(standard8) → review+
Updated•10 years ago
|
Assignee: nobody → nperriault
Assignee | ||
Comment 5•10 years ago
|
||
Rebased patch on top of latest fx-team; as this involved revamping part of the API (eg. getInitialStoreState), requesting a new review here.
Attachment #8517952 -
Attachment is obsolete: true
Attachment #8524695 -
Flags: review?(standard8)
Assignee | ||
Comment 6•10 years ago
|
||
Dead code & obsolete jsdocs removal.
Attachment #8524695 -
Attachment is obsolete: true
Attachment #8524695 -
Flags: review?(standard8)
Attachment #8524838 -
Flags: review?(standard8)
Assignee | ||
Updated•10 years ago
|
Points: --- → 2
Comment 7•10 years ago
|
||
Comment on attachment 8524838 [details] [diff] [review] Create a shared store creator for Loop. Patch v4.1. Review of attachment 8524838 [details] [diff] [review]: ----------------------------------------------------------------- Looks good.
Attachment #8524838 -
Flags: review?(standard8) → review+
Comment 8•10 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/60a876878c55
Iteration: --- → 36.3
Target Milestone: --- → mozilla36
Updated•10 years ago
|
Whiteboard: [tech-debt]
https://hg.mozilla.org/mozilla-central/rev/60a876878c55
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment 10•10 years ago
|
||
Comment on attachment 8524838 [details] [diff] [review] Create a shared store creator for Loop. Patch v4.1. Approval Request Comment [Feature/regressing bug #]: N/A Required for Rooms support in 35 [Describe test coverage new/current, TBPL]: on m-c, rooms is now enabled on Nightly [String/UUID change made/needed]: none
Attachment #8524838 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
Attachment #8524838 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in
before you can comment on or make changes to this bug.
Description
•