- Merge request !877Issue #2708101: Default value for link text is not saved → (Closed) created by bnjmnm
- 🇭🇺Hungary csakiistvan
@renukakulkarni for me this patch is is very nice, but the link field will mandatory on the form display (but my field is not mandatory on the field settings.)
- 🇺🇸United States emerham
At least, for me, It seems unusual to have default text for different URLs.
Also using field tokens you can have a mostly static title that replaces part of the text on render.
For instance we have some PAC classes and we want the link to be:
View the OSU course catalog page for [node:field_pac_class_name]
.
As long as you make the token field required then it should work. Tested on a site that I'm migrating and it works with this patch. - 🇺🇸United States DamienMcKenna NH, USA
FYI there's a related feature request for allowing a static link title: ✨ Link field: add the ability to have a static anchor text and link title Active
- 🇺🇸United States DamienMcKenna NH, USA
The patch mostly works. However, if a default title is provided, when trying to save the entity form the URL field will be required, there's no way of saving the field without an URL. Which is what @csakiistvan said in #121.
- Status changed to Needs review
about 1 year ago 9:09am 8 December 2023 - 🇫🇷France pguillard
Nice patch, I agree with #125.
I guess it's worth a Needs review, if not RTBC yet. - Status changed to Needs work
about 1 year ago 9:49am 8 December 2023 The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇺🇸United States douggreen Winchester, VA
I rebase and force pushed the branch. It needs review by someone who is familiar with what this does.
- First commit to issue fork.
- 🇮🇳India shalini_jha
I have replicate this issues and after applying the changes in the MR 8552, the link text default value save is working properly.
I have observed the pipeline failure, and i have fixed the pipeline failure and now all the test passes in MR 8552 against 11.x . same branch have MR 877 against 9.x , need to hide this MR as we are using MR 8552.Apart from that i am also agree with #125 . but according to #126 i have fixed the pipeline failure and moving for NR.
Kindly Review. - 🇪🇸Spain juanolalla
From #125 🐛 Default value for link text is not saved Needs work :
The patch mostly works. However, if a default title is provided, when trying to save the entity form the URL field will be required, there's no way of saving the field without an URL. Which is what @csakiistvan said in #121.
From #121 🐛 Default value for link text is not saved Needs work :
For me this patch is is very nice, but the link field will mandatory on the form display (but my field is not mandatory on the field settings.)
I see there is still some confusion with the natural core's behavior of the form display, forcing an URL to be set if there is a title set, which is correct, and the issue we are fixing here, just allowing a default title text without a default URL in the field settings form.
We had that explained in #82 🐛 Default value for link text is not saved Needs work :
The review in #79. is based on an incorrect understanding of the requirements.
Expected Results:
# User should be able to Save the node without mentioning the URL field (as URL field is non required)That is incorrect. It's actually a good thing that this is happening. This issue is to allow a default link text value without including a default URL value for a field. When the field is in use, it should still enforce the presence of a URL value if the link text is not empty.
So please, let's not bring again the form display behavior that is in core, which is correct, and it is not part of the issue we are dealing with here.
- 🇪🇸Spain juanolalla
There are no interface changes with this issue, it's just a bug: the link text was not saved, and now with the fix it is saved.
So this doesn't need usability review.
No remaining tasks and nothing else, so moving this to RTBC.
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- First commit to issue fork.
- 🇮🇳India mrinalini9 New Delhi
Fixed phpstan issues reported by the bot and have updated the MR, please review it.
Thanks!
- 🇮🇳India sagarmohite0031
Hello,
Verified and tested on Drupal 11.
The MR is applied successfully.1.Add a link field to a content type.
2.The link field is configured with "Allow link text" set to be required.
3.Under Default Value, enter text for the "Test data for link".
4.Leave the "default URL" empty.
5.Save the field configuration.
6.Edit the link field and see that the default link text should saved.Working as expected -
Default link text is saved successfully.Check attachments-
- 🇳🇿New Zealand quietone
With the settings shown in the screenshot, before this change the link field is not required and after this change it is, even though the 'Required field' setting is not selected. I think that will cause confusion for admins and content editors, as it has done for people working on this issue. I think we find a way to resolve that here. For that getting an Usability review will help, so I am tagging and changing status.
- 🇪🇸Spain juanolalla
With the settings shown in the screenshot, before this change the link field is not required and after this change it is
That is not true, please see #135 for the explanation. This fix is not changing at all the current's Drupal core behavior, that requires an URL if you would like to set a link just with a link text.
This is just a simple fix for the existing bug on the field settings form: the default link test it is not being saved.
This fix has nothing to do and touches nothing of the display form. Please open a new ticket to discuss current Drupal's behavior from an usability point of view, and let this fix solving the bug be committed.
- 🇳🇿New Zealand quietone
Yes, I read #135 and the other comments as well. Yes, this is a bug with a straightforward fix. That fix, however, has caused unexpected behavior as noted in #79, #101, #116, #121, #125 and #143.
And this fix does cause changes the UI, on a node/add form. The URL part of link field will be shown to the user as required. That is the change and yes it make sense. However, there is no information provided to let the use that the form can be submitted without completing the required field. That is why I asked for a usability review here 3 years ago and I still think one should be done.
- 🇷🇴Romania amateescu
I think it would be easier to introduce a field item list class for the
link
field type and override\Drupal\Core\Field\FieldItemList::defaultValuesFormSubmit()
to manually return a default value if thetitle
setting is set to required and there's a value for the link title. - 🇪🇸Spain juanolalla
And this fix does cause changes the UI, on a node/add form.
No, this fix doesn't cause changes in the UI. What you are referring is not part of this fix, is Drupal core current's confusing UI.
Right now, if you want to save a default value for the link text, that's broken. This patch is allowing many sites to fix that, for years. It should be committed.
The URL part of link field will be shown to the user as required.
This is the current core behavior, not the result of this fix. If you try to save a link field with text and no URL. Please open a new ticket to improve that.
- 🇪🇸Spain juanolalla
I think it would be easier to introduce a field item list class for the link field type and override \Drupal\Core\Field\FieldItemList::defaultValuesFormSubmit() to manually return a default value if the title setting is set to required and there's a value for the link title.
I don't understand how that would solve the current problem of the UI's bug, failing to save the default value in the field settings form. And why are you changing the status of this bug fix to "needs work" again.
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
I'm not sure about the fix here. I think saving an invalid default value might result in more problems as all other default values are valid and not empty. I wonder if we should consider a different fix where we save the default title value somewhere else in the field config if the link is not present. That way we wouldn't have to override extractFormValues which feels a bit risky.
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
@juanolalla this does result in a behaviour change and it will be confusing for users because once you have a default link title it's completely not obvious that you could remove the link title to save the content without a link. This suggests a potential fix - only allow setting a default link title if the field is required.
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
On posting the comment I do think we could consider a completely different fix. We could only allow a default link title to be set if the default link is also set. I've read the issue summary and I've not seen a use-case for providing a default link title without the link url. The link title and URL have to be related right - so the user needs to make the title fit the link.
Also as far as I can see the use of titles on links is largely discouraged - see https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/titl..., https://www.tpgi.com/using-the-html-title-attribute-updated/ and https://www.24a11y.com/2017/the-trials-and-tribulations-of-the-title-att... I don't think providing a default title with no related link is going to make this situation better.
- 🇪🇸Spain juanolalla
Also as far as I can see the use of titles on links is largely discouraged
@alexpott This issue is about the default link text, not about link title attribute.
I've read the issue summary and I've not seen a use-case for providing a default link title without the link url.
@alexpott We are using this patch in many sites. The client needs to set a default link text so it is always the same text and the URL is changing: let's say for example "See the full source for more details", where the link pointing to the resource is different in every node.
this does result in a behaviour change and it will be confusing for users because once you have a default link title it's completely not obvious that you could remove the link title to save the content without a link.
The way I see this, more than "this does result in a behavior change", is that this actually allows something expected, the ability to save a default link text (instead of the frustration of doing it and see that it is not saved), and as you say, allowing the expected functionality allows the situation of a new node with a link text already filled and the user could be kind of forced to fill either the URL or delete the link text for that specific node.
However having to delete the link text to leave it empty so it passes validation without a URL in a non required field scenario, is something core is doing right now, not this fix. This fix is just removing a veil over an existing problem in core, hidden by the bug of the default text not actually being saved, despite the UI's supposedly allows it as with any other field. That's why I am encouraging to open a new issue about this, tackling what core does in the form display with links and URLs not filled on non required fields.
This suggests a potential fix - only allow setting a default link title if the field is required.
While it could be the most common scenario, we would not be satisfying all possible desired functionalities. I, as client, could perfectly desire to have a default link text for this field in all nodes that have an URL, which maybe is the 80% of the nodes for that content type, but not 100% so I don't want it to be required, but the default link text is a must, for consistency and time saving.
The real problem here is that, right now, core forces you to remove the link text if you write something on it and click save without filling the URL. This is the issue we are talking about now, the one unveiled by fixing this bug, +8 years open issue.
A possible solution to current's core not very good behavior, could be maybe to let the user have clear that the link text is ultimately dependent on the URL on a specific node edit/create form. So for example, if the link text input would be hidden and only be shown when the URL is actually filled, that would make things clearer, and possibly we wouldn't have to make the user delete the link text input if she/he finally saves the node without a value for the URL, as core currently enforces. As @quitetone says, that would require an usability look.
My proposal is to open a new issue from here to improve current's core confusing behavior, instead of leaving this bug unfixed forever so we not have to look at what is actually an incomplete implementation. If we decided not to fix this bug until we have a better display form implementation we should at least alert the user than if she/he takes the time to fill a default text link value, she/he should also fill an URL default value, or directly remove the option of filling a default text link. But I would love not to do these things, so many people as myself could at least be patching (and maintaining, rerolling) this to allow this necessary feature to our clientes. Many people are actually needing to have this failing feature solved.