blackforestboi
commented
4 years ago @cdharris thanks for the write up! 🎉
@poltak It does not seem to be super clear from the description what the priorities are (and is not the goal of it to communicate them) so it's important to note that we want to start with the collections picker the tag filter comes later. It should be an easier path for now and is more urgently needed for consistency reasons.
The places where the collections picker shoould be:
- the ribbon
- the browser popup (top right corner)
- NEW in the result element with a new button (just like the tag picker)
@chris
cdharris
Tags Picker - Extend for Applying tag filters and Collection Picker
This task was split off from the main Tag Picker feature in https://github.com/WorldBrain/Memex/pull/985 and https://github.com/WorldBrain/Memex/issues/948 as it starts to require some changes to make the component more generic, and it might be useful to use this as a stepping stone into architecting the Collection Picker.
This involves:
[ ] There's no ability to add a new tag
Options I see to adapt for this:
1) Adding this functionality to the existing tag picker, behind conditional properties. Starts to mix up the logic - not preferred, it's one of the things we were seeking to get rid of in favour of more single purpose utilities.
2) Copying the TagPicker and modifying it to be a TagFilter - probably the fastest option and cleaner than the 1st option, but would have a lot of duplication of code if just done as is. Some duplication is fine if it aids readabiliy and maintanance, but we want to avoid duplication that makes it harder to maintain and fix underlying bugs, etc.
3) Extracting out the generic components and logic across TagPicker and TagFilter (and CollectionPicker) that can be composed to reduce duplication. This will end up with a component that is cleaner and contains less duplication, but because of the additional moving parts, may be a bit more involved to read and maintain.
A good approach is probably to start with 2. Copying the main TagPicker logic and top level component into a TagFilter, and Collections Picker, and then start refactoring the code to share logic and components as needed, being mindful not to
Verdict: To aim for 3., falling back to 2. with elements of 3. And doing this work as part of the Collection Picker work, as it will be a nice segway into extracting the TagPicker logic to be more general and composeable.
Development Patterns
Some patterns and approaches that have worked out in the tag picker I reccomend to follow (that will make it into component guide for the future):
Keeping all business logic out of sub components and avoiding as much as possible doing any calculations on render.
This has allowed really comprehensive UI tests to be written. It sometimes requires formating the internal state data structure in a way to best serve the UI, for example keeping the
focusedorselectedstate as properties of aDisplayTaga front-end only type, or havingshowNewTagbutton as a precomputed property in the state, rather than making that decision based of a condition somewhere. Modifing these in the UI Logic of the controlling component, rather than keeping local component state makes testing much easier and also isolates computation so performance can be tackled in one place.Where possible, composing components in the parent component render rather than in sub components.
E.g.
TagSearchInputcan take aTagSelectedList, orTagResultListcan take aTagResultRow, this avoid passing through props up and down the tree, and the interface to the logic can be kept in the main component.see https://github.com/WorldBrain/Memex/blob/24d478aa2006e8166836062666f8050f68ee4422/src/tags/ui/TagPicker/index.tsx#L60-L121
Oppertunistic operations where possibe.
E.G. The component makes changes and updates itself assuming the operation has worked - we don't wait syncronously for the confirmation to come back from the storage layer and block or slow the UI, preventing other operations while this happens. If the result fails, then we set the state back accordingly and inform the user. This makes the UX feel super quick.
see https://github.com/WorldBrain/Memex/blob/24d478aa2006e8166836062666f8050f68ee4422/src/tags/ui/TagPicker/logic.ts#L407-L427