Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
When adding a text field with multiple values, the text field will overflow over its designated area on narrow screens (the minimum width this happens is 780px).
Proposed resolution
Port the solution from Claro, and change .form-element
to .form-text
/**
* Target every .form-element input that parent is a form-item of a table cell.
* This ignores the filter format select of the textarea editor.
*/
td > .form-item > .form-element,
td > .ajax-new-content > .form-item > .form-element {
width: 100%;
}
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#29 | interdiff_27-29.txt | 871 bytes | ravi.shankar |
#29 | 3151119-29.patch | 509 bytes | ravi.shankar |
#27 | interdiff-3151119-23-27.txt | 788 bytes | msuthars |
#27 | 3151119-27.patch | 793 bytes | msuthars |
#23 | interdiff-18-23.txt | 514 bytes | Hardik_Patel_12 |
Comments
Comment #3
shaalComment #4
shaalScreenshot after applying the patch:
Comment #5
shaalRerolled patch for 8.9.x (no interdiff)
Comment #6
shaalComment #7
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedComment #8
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedVerified and tested by applying patch #5.It is working fine for D.8.9.Can be moved to RTBC.
Steps to test -
1. Go to the admin site.
2. Go to admin/content.
3. Search for the recipe content and edit it.
4. Verify
Comment #9
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedComment #10
shaal@priyanka.sahni Thank you for reviewing the patch.
If the patch was working for you and did what it suppose to, you can change the status of this issue to RTBC.
Comment #11
shimpyPatch #5 looks good to me as working as per functionality. Here attaching screenshots for same and changing status to RTBC
Before patch
After Patch
Comment #12
lauriiiI'm wondering how does this play if someone has a text field that has size higher than 60 characters? I'm also concerned about some browsers not respecting max-width for form inputs.
Comment #13
Kristen PolTagging this issue for new contributors to work on during the Friday DrupalCon Global sprint.
Comment #14
Kristen PolComment #15
rakhimandhania CreditAttribution: rakhimandhania as a volunteer and commentedComment #16
imalabyaTested the patch in various browsers (Chrome, Firefox, Safari, Edge & IE11) Works well. The text fields doesn't overflow.
However, I don't think the
max-width: 62ch
is redundant. Also, in IE11 the text field doesn't take the full width of the container.With
max-width: 62ch
Without
max-width: 62ch
IMO, we can remove the
max-width: 62ch
from the PR.Comment #17
harika gujjula CreditAttribution: harika gujjula at Specbee for Drupal India Association commentedComment #18
harika gujjula CreditAttribution: harika gujjula at Specbee for Drupal India Association commentedI have removed the max-width set and tested. This works fine.
Please find the updated patch.
Comment #19
Kristen Pol@harika gujjula Thank you. When you update patches, it is best to include an interdiff:
https://www.drupal.org/documentation/git/interdiff
Though in this case it is ok since the patch is so small.
Comment #20
HarishSTApplied the patch in #18 and works as intended. Attached the screenshots by testing on Firefox, Chrome and IE11.
Thanks @harika, for the patch.
It looks good to me. Moving to RTBC.
Comment #21
yoroy CreditAttribution: yoroy at Roy Scholten commentedTested this with a new multi-value text field item on the article content type. Works as described, so this seems like a good fix.
One niggle, this code comment reads a bit awkward, not sure it's super correct:
> Target every .form-text input that its parent is a form-item of a table cell.
Should that be something like
> Target every .form-text input where its parent is a form-item inside a table cell.
?
Comment #22
Kristen PolThanks for the reviews. Back to needs work for comment change.
Comment #23
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedUpdating the comment , kindly review.
Comment #24
samiullah CreditAttribution: samiullah at Salsa Digital commentedComment has been added as expected
Target every .form-text input where its parent is a form-item inside a table cell. */
@kristen.pol this can be moved to RTBC if no other code review is needed
Thanks
Comment #25
Kristen PolThanks for the update and testing. A couple nitpicks:
1) Exceeds 80 characters.
2) The comment style switched from:
to
It appears the latter style is normally used for inline comments (after the CSS), or sometimes if the comment is only one line long. Since the comment exceeds 80 characters, if that was changed, it would be more than one line and should switch to the former format.
Comment #26
msutharsComment #27
msuthars@Kristen Pol thanks for the suggestions. I updated the patch. Please review.
Comment #28
Kristen PolThanks for the update.
Changes here are out of scope for this issue.
Changes here are out of scope for this issue.
This doesn't address the second part of the comment in #25:
If this is not clear, it needs to change to using this formatting:
Comment #29
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedHere I have addressed comment #28.
Comment #31
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedChanging status to needs review as patch #29 passed the tests.
Comment #32
Kristen PolThanks for the update.
1) Patch applies cleanly.
2) Tests pass.
3) Code was manually tested in #20 and #21 so I don't think it needs retesting. Subsequent changes were to comments.
4) CSS was reviewed in #12 and #16, and concerns were addressed by removing
max-width: 62ch
. Subsequent changes were to comments.5) #29 addresses items from #28.
6) Although my inclination would be to try to get the text down to a one-line comment (since it spills over by only one word), the wording is what was suggested in #21. I'm not sure if this needs an automated test or needs testing on other themes, but marking RTBC to get maintainers thoughts.
Comment #34
Hardik_Patel_12 CreditAttribution: Hardik_Patel_12 at QED42 commentedRandom failure , setting back to RTBC.
Comment #36
alexpottCommitted c5489de and pushed to 9.2.x. Thanks!
Going to discuss with @lauriii about whether we should backport this to 9.1.x
Comment #38
alexpottDiscussed with @lauriii we agreed to backport this to 9.1.x.