Updated: Comment 0
Problem/Motivation
drupalSettings.states never clears, it's very noticeable on the views UI where #states is used all over the place. It gets slower and slower to open modals. Because all the rules just adds up in drupalSettings.states (It's easy to arrive to 120 rules after opening a few modals). and they all get processed every time even if the elements don't exist on the page.
Also, very slow. Event delegation on the views UI modal reduce the time it takes to open a modal by 1 second (!) on desktop (from 3.8s to 2.9s).
Proposed resolution
This patch move the #states configuration away from Drupal.settings and into a data-drupal-states
attribute on the relevant form element (it's data-drupal-states because data-states sounds like it'd be an easy naming confilct).
I had to move the drupal_process_states() call before drupal_render to add the attributes from this function.
The JS change is really straightforward, instead of getting the values from Drupal.settings it takes it from all the elements having a data-drupal-states attribute. The #states configuration is serialized in json inside the attribute. It's legit from a HTML standpoint, anything goes inside data-* attributes.
Remaining tasks
User interface changes
API changes
Related Issues
Original report by @nod_
Comment | File | Size | Author |
---|---|---|---|
#14 | states_for_type_item_broken-1589176-14.patch | 1.01 KB | Wim Leers |
#7 | core-js-data-states-1589176-7.patch | 2.63 KB | nod_ |
core-js-data-states.patch | 2.37 KB | nod_ | |
Comments
Comment #1
nod_forgot tagging
Comment #2
nod_Comment #3
nod_1 year and no feedback, probably not such a good idea :p
Comment #4
klonos...or perhaps not enough time. Still a good idea though (since I see no negative comments stating the opposite). Let's see if we can do it in D9 then ;)
Comment #5
nod_It's easy enough to put in 8.x it's just that I see no justification for it really.
Comment #6
nod_Now I see it.
drupalSettings.states never clears, it's very noticeable on the views UI where #states is used all over the place. It gets slower and slower to open modals. Because all the rules just adds up in drupalSettings.states (It's easy to arrive to 120 rules after opening a few modals). and they all get processed every time even if the elements don't exist on the page.
Also, very slow. Event delegation on the views UI modal reduce the time it takes to open a modal by 1 second (!) on desktop (from 3.8s to 2.9s).
Comment #7
nod_There is also the problem that the script works kind of backwards, but we can't fix everything at once.
The following patches cut down 800ms from init of Views UI modals #1851414: Convert Views to use the abstracted dialog modal
Comment #8
nod_actually having leftover settings from elements that have been removed from the page in drupalSettings is a bug.
Comment #9
dawehnerManual testing of some bits of the views UI still works and I have to admin that it feels way smoother, a little bit more like D7 to be honest.
This variable name really seems odd. What about using states_length instead?
Updated the issue summary.
Comment #10
nod_the variable name is used all over the place in the rest of the scripts so I don't have anything against changing it but it'll be the odd one out.
Comment #11
dawehnerOkay fine, if this is a standard.
Comment #12
catchCommitted/pushed to 8.x, thanks!
Comment #14
Wim LeersThis broke
#states
for#type => item
!Before this commit, unchecking
admin/config/content/formats/manage/basic_html
's "Enable image uploads" checkbox correctly hid the "Maximum dimensions" (which is#type => item
) form item, but since this commit, that's broken.The reason is that for
#type => item
elements,#attributes
aren't used (because there's only a wrapper and not an actual HTML form input element), which is precisely what's necessary since this patch for#states
to work.However,
#wrapper_attributes
does always get set, and in this case that's in fact more appropriate (i.e. only a wrapper, no actual HTML form input element on which#attributes
could get set). Patch attached.Ideally, we'd have test coverage that would've prevented this regression, but that's something sun should've done years ago, when this exact same bug was first encountered at #783438: #states doesn't work for #type item. In fact, AFAICT we have zero test coverage for
#states
non-JS side of things, as demonstrated by the committed patch. So there are no tests I can extend.Comment #15
nod_Tested and it works. Not pretty but that's just how it is.
this makes the code for #2033959: Improve design of language detection and selection settings page a bit cleaner (which is why i know it works).
Comment #16
nod_Comment #16.0
nod_blub
Comment #17
webchickCommitted and pushed to 8.x. Thanks!
Comment #18
Wim Leers