Closed (fixed)
Project:
Token
Version:
8.x-1.x-dev
Component:
User interface
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
9 Dec 2016 at 16:40 UTC
Updated:
27 Dec 2016 at 16:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
VladimirMarko commentedComment #3
aaronbaumanI came here to post a patch for the same issue.
Let's get it fixed!
Did you assign to yourself because you're working on a patch?
Otherwise, you should unassign to indicate that it's ready to be picked up by anybody.
Comment #4
berdirHe did, but he also left for the weekend already. If you have a patch, post it.
Comment #5
aaronbaumanok, here it is
Comment #6
berdirThat's... an interesting use of #description.
Can you post a screenshot of how that looks?
Also needs tests
Comment #7
aaronbaumanI was surprised that this worked at all:
The 'browse' link is appended to div.description
The problem with a more standard approach is that "Title" field doesn't have a #weight during form_alter, so it's not straightforward to situate a new element between the first 2 fields.
This approach was more expedient than, e.g. implementing a process callback.
What would a test look like for this?
There aren't any tests for the other 'browse' link implementations for me to crib.
Comment #8
berdirThat looks quite nice. thanks. The problem with it might be that it won't work so well for a right to left language like russian. But probably not a huge deal if the link is before the text.
We have \Drupal\token\Tests\TokenBlockTest already. So basically, what we need to do is on the page where we edit the block, that we add an assertLink() that makes sure the link is displayed there. So it should be like a single additional line.
Comment #9
VladimirMarko commentedI am missing the space between "tokens." and "Browse", when I apply the patch.
It says "This field supports tokens.Browse available tokens."
Comment #10
berdirYes, I suspected this would happen.
We could try something like t('This field supports tokens. @browse_tokens_link') and then pass in the render array like that, but we probably need to render it ourself then with \Drupal::service('render')->render()
Comment #11
VladimirMarko commentedFixed by using a placeholder as suggested.
There was no need to use render().
Also updated the associated test with two lines of assertions.
Comment #14
VladimirMarko commentedAdded the \Drupal::service('renderer')->render() call. There are no more exceptions in my local test.
Comment #15
VladimirMarko commentedForgot the test only patch. Here it is.
Comment #18
berdirComment is too long.
Remove @todo left-over.
remove the old commented out line.
use short array syntax
#prefix stuff isn't needed, just = t(...);
Comment #19
VladimirMarko commentedI made the required changes.
Comment #22
aaronbaumanNice, looks good to me.
Comment #24
berdirAgreed, committed.