On sites with a lot of tokens opening a page with the token replacement list will slow down the browser or even crash it.
The token module lately introduced the possibility to show tokens in a dialog which loads the tokens on demand using ajax: #1684984-17: Support opening token browser in a jQuery UI dialog on demand
The change is backward compatible.
Btw. loading the tokens in the dialog is still slow and still could crash the browser but at least it would be on demand :D
Review patch
I'm not sure if this plays well with accessibility concerns!
The tree list of tokens is replaced by a link to open a dialog with the token tree list.
none
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | token-link.png | 74.78 KB | quicksketch |
| #8 | webform-token_dialog-1801782-8.patch | 3.2 KB | quicksketch |
| #6 | webform-token_dialog-1801782.patch | 1.21 KB | aaronbauman |
| webform-use-token-dialog-if-available.patch | 320 bytes | das-peter |
Comments
Comment #1
das-peter commentedComment #2
wiifmPatch is simple, applies cleanly, and works as expected. This is a must do.
Comment #3
aaronbaumanRecategorizing as a bug.
On a medium-size site, the large volume of tokens is actually causing a fatal memory error (256M exhausted).
Yes, the root cause is an issue in token, but it must be solved in webform.
This is a one-liner, let's get it in.
Comment #4
quicksketchThanks guys. Still RTBC, but it's not a bug in Webform. I'll put this in next time I'm going through the Webform queue.
Comment #5
quicksketchThis patch UI-wise doesn't make sense that we would have a collapsed fieldset with nothing but a link in it. We should append the token browser link to the existing description text. The token browser itself leaves a lot to be desired still (the fact that it doesn't have a scroll bar for instance), but that can be handled in the Token project.
Comment #6
aaronbaumanOK, here's an updated patch for theme_webform_token_help() to return a div instead of a fieldset.
I also added a selector class=webform-token-help to the div, since this patch changes the DOM.
Looks like this:
Comment #7
das-peter commentedMade a visual review. Looks good to me, I claim it's RTBC ;)
Comment #8
quicksketchFor consistency and aesthetics, I moved the token help out of a dedicated element and added it to the description text of the field that supports the tokens. This makes our settings page consistent with the configuration of individual components, which show the token help in the description text for each field. This approach also falls back to using the previous fieldset if the token dialog is not available.
Comment #9
quicksketchScreenshot of modified link location:
Comment #10
quicksketchThanks guys. Finally committed to 4.x.