Problem/Motivation
There is an option to set #dialog to FALSE
when calling theme('token_tree')
or theme('token_tree_link')
. By default, it is TRUE and all code within token module sets it to TRUE anyway. Setting it to FALSE throws an error and fixing the error does not change anything (except change the look of the dialog slightly). In other words, there is a dialog no matter the value of #dialog
.
Proposed resolution
This brings up a question? What is the point of the variable? Is it an outdated functionality we no longer use? If yes, we should remove it. If not, we should fix it to whatever behaviour.
Remaining tasks
- Remove the dialog variable.
OR
- Fix it to whatever behaviour we need.
User interface changes
None
API changes
Possibly removing the dialog variable.
Data model changes
None
Comment | File | Size | Author |
---|---|---|---|
#15 | dialog_option_for_token-2640170-15.patch | 2.96 KB | hussainweb |
|
Comments
Comment #2
hussainwebI am happy to write the patch for either of the scenarios. I just wanted to know from the maintainers what is the behavior intended.
Comment #3
BerdirI'm confused. What you are saying here seems to contradict #2640138: Use token_tree_link instead of token_tree to render links, where you remove the option, which is the same as setting it to FALSE? I tried, it works fine for me.
Comment #4
hussainwebI think the IS was a little confusing. If you set #dialog to FALSE for theme('token_tree'), it will just render the entire token tree, which is fine. But if you set #dialog to FALSE for theme('token_tree_link'), then you would see an error 'Unsupported operand types'. That error is just a small one-line fix and the code moves ahead. However, you will still get the same link and a similar dialog which works but doesn't look the same (due to missing libraries).
And I am not really removing the #dialog option from theme_token_tree_link in #2640138: Use token_tree_link instead of token_tree to render links. I am just removing it from theme_token_tree where the option doesn't make sense, IMHO. I intend to remove the #dialog option here, if we are not using it. The other issue is orthogonal to this, not contradictory.
Comment #5
BerdirAh, yes. lets remove #dialog from token_tree_link.
Comment #6
hussainwebAttaching a patch which removes #dialog. As always, I tested it manually to see that the link and dialog is still working on different pages.
Comment #8
hussainwebAh, I created the patch over the changes in #2640138: Use token_tree_link instead of token_tree to render links. If that is not being committed soon, I will create a new patch here.
Comment #9
BerdirComment #10
hussainwebGreat! All passed. Thank you for committing the other patch. :)
Comment #11
Berdirthis change seems wrong, that overwrites the token-dialog class that you set above?
Comment #12
hussainwebSorry, I had done that for testing a related issue. I'll check and revert that.
Comment #13
hussainwebFixing for the issue in #11.
Comment #14
BerdirThat description doesn't make sense. It's the regular link part that we're removing and it still says either but only hase one case?
"Theme a link to a token tree shown as a dialog." ?
Comment #15
hussainwebYes, I didn't realize it. I changed it as per your suggestion. I skipped the interdiff (that's the only change). I hope that's okay.
Thanks for the reviews. :)
Comment #16
BerdirThanks, committed.