Problem
Currently, when a link field's "title" element is set to DRUPAL_OPTIONAL, a user can enter link text, leaving the link URI blank, and the form will save, while not saving any link data (since the field is considered empty). This poses a usability problem: users entering content will feel like it "disappears."
Proposed Solution
Add widget validation when the title element is set to DRUPAL_OPTIONAL: if a title is entered but no link, notify the user of this with a form error, rather than silently saving without the field data.
Comment | File | Size | Author |
---|---|---|---|
#18 | validate-title-no-link_2923496_18.patch | 3.37 KB | dpfitzsi |
#10 | validation-link-text-no-link.png | 82.39 KB | mark_fullmer |
#10 | validate-title-no-link_2923496_10.patch | 2.76 KB | mark_fullmer |
#8 | With Patch Validation.png | 38.18 KB | krina.addweb |
#8 | With Patch Editing the content.png | 48.3 KB | krina.addweb |
Comments
Comment #2
mark_fullmerThe attached patch provides said validation and refactors the existing LinkFieldTest to verify this validation behaves as expected.
Comment #3
rang501 CreditAttribution: rang501 at ADM Interactive commentedHi!
This looks really good improvement and works similarly to the opposite validation (title required), but I think the error should be set on the Url field, not Title.
Testing details
In my opinion, the Title field should be normal and Url field highlighted with error.
Comment #4
rang501 CreditAttribution: rang501 at ADM Interactive commentedComment #5
mark_fullmerAlright, rang501, the attached patch sets the form error on the URI rather than the title.
Comment #7
mark_fullmerComment #8
krina.addweb CreditAttribution: krina.addweb at AddWeb Solution Pvt. Ltd. commented@mark_fullmer, Your patch works for me as mentioned in "Proposed Solution"
I Checked your patch by following steps:
> Add Link Field to Article Content Type.
> Add Link Title text & leaving Blank the URL field.
> It fires the Validation to add the value in URL
> Without adding both values, It didn't fire any validation.
> Also, It saves the details while editing the content.
Comment #9
xjmThank you for working on this! Glad to see test coverage for the fix, and thanks for the screenshot which is very helpful.
Shouldn't the validation be flagging the URL field, since that's where the error is? Or both of them?
For the message, I'd suggest:
Also I'm not sure the URL field should be marked required during this validation step. If the field itself is not required, both could be left blank, but this validation might confuse the user into thinking they must enter something when it's also an option to leave both fields blank.
Thanks!
Comment #10
mark_fullmerThanks for the testing, krina.addweb, and thanks for the feedback, xjm. Patch #10 adds the following changes, per xjm's suggestions:
Comment #11
mark_fullmerComment #12
mark_fullmerComment #14
mark_fullmerComment #15
lreynaga CreditAttribution: lreynaga at University of Texas at Austin commentedI can confirm that patch in #10 works as described. Code looks solid. Looking at the latest build in Jenkins everything passed, including the LinkFieldTest class (16:05:27 Drupal\Tests\link\Functional\LinkFieldTest 6 passes).
I feel confident enough to consider it reviewed.
Thanks!
Comment #16
lreynaga CreditAttribution: lreynaga at University of Texas at Austin commentedComment #17
catchI had to read this comment a couple of times, would this be clearer:
"Ensure that a URI is always entered when an optional title field is submitted."
Comment #18
dpfitzsi CreditAttribution: dpfitzsi as a volunteer and commentedUpdated patch to incorporate catch's comments.
I have also added this same functionality when the title field is required. If you just enter a title and try and submit the form will still submit. So this functionality really shouldn't only apply to optional title fields.
Comment #19
lreynaga CreditAttribution: lreynaga at University of Texas at Austin commentedTested #18 this time with both an optional and required title text, both cases were behaving as described. Moving to RTBC.
Comment #20
lreynaga CreditAttribution: lreynaga at University of Texas at Austin commentedComment #21
dpfitzsi CreditAttribution: dpfitzsi as a volunteer and commentedThanks lreynaga for testing the patch!
Comment #24
catchCommitted/pushed to 8.6.x and cherry-picked to 8.5.x. Thanks!