Fixes T4006.
Details
- Reviewers
epriestley - Maniphest Tasks
- T4006: Maniphest batch editor clears selected style when you drag tasks to a new priority
- Commits
- Restricted Diffusion Commit
rPb354ef7aa905: Maniphest - fix a bug when selecting all tasks and then dragging them around
clicked "select all" and dragged around tasks. Noted the task remained selected as I re-ordered, thus keeping hte count accurate. Verified when I hit "batch edit" the right tasks showed up.
Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
It's intentional that JX.DOM.hasClass() does not exist, because this hasClass() junk caused a serious and avoidable privacy bug (and some various other nonsense) at Facebook.
Javelin uses sigils instead of CSS classes to rigidly enforce the difference between semantic information and style information in the document. While CSS classes can theoretically handle both, the conflation between semantic and style information in a realistic engineering environment caused a number of problems at Facebook, including a few silly, preventable, and unfortunately severe bugs.
Javelin separates this information into different namespaces, so developers and designers can be confident that changing CSS classes and presentation of a document will never change its semantic meaning. This is also why Javelin does not have a method to test whether a node has a CSS class, and does not have CSS selectors. Unless you cheat, Javelin makes it very difficult to use CSS class names semantically.
http://www.phabricator.com/docs/javelin/article/Concepts_Sigils_and_Metadata.html
Instead, you could write a flag into the node data when the thing is selected:
JX.Stratcom.getData(node).isBatchSelected = true;
...although this will be kind of messy.
We could also merge the maniphest-subpriority-editor and maniphest-batch-selector behaviors; there is little reason for them to be separate now.
Another alternative would be for this behavior to emit an "updated a node" event and have the batch selector listen for it and repaint the correct selection state.
went with the event suggestion. I think the separation of behaviors is still good per the simple behavior thing. I think the updating the node data suggestion was less good than this.