Problem
In order for Drupal to be WCAG 2.0 compliant, compound form elements (radios and checkboxes, for instance) should be wrapped into fieldsets. However, wrapping these controls into fieldsets has an undesirable effect from a usability point of view: it adds a visual border around the control.
Proposed resolution
Add a CSS class that can be used to hide borders from elements such as fieldsets and table headings.
Comment | File | Size | Author |
---|---|---|---|
#86 | Screen Shot 2014-02-10 at 11.35.26 AM.png | 540.95 KB | mgifford |
#84 | User-1-Edit.png | 149.51 KB | mgifford |
#84 | Configure-Site-Screen.png | 551.11 KB | mgifford |
#77 | installer-fieldgroup.png | 35.78 KB | sun |
#77 | drupal8.fieldgroup.patch | 3.25 KB | sun |
Comments
Comment #1
falcon03 CreditAttribution: falcon03 commentedComment #2
Liam MorlandThe latest patch in #504962: Provide a compound form element with accessible labels removes the default border on fieldsets.
Comment #3
falcon03 CreditAttribution: falcon03 commented@Liam Morland: does it remove borders from fieldsets using a class or just redefining the "fieldset" tag appearance?
Maybe we should introduce the CSS class in this separate issue, because it will be useful for other things than fieldsets too: for instance, adding tableheadings to the status report.
Comment #4
Liam MorlandWith the other patch installed, fieldsets do not have a border by default. Basically, it is:
fieldset { border: 0; }
Comment #5
falcon03 CreditAttribution: falcon03 commented@Liam Morland: since we need to hide borders also from tableheadings, do you think we should re-define the html elements any time instead of assigning a CSS class to them?
I don't know what the best option is.
Comment #6
Liam MorlandI think it is best for the default CSS to have no borders and for a class to add borders when they are needed. Maybe tables should be an exception, since they normally have borders. A table without a border would be an exception.
Comment #7
ry5n CreditAttribution: ry5n commented+1 to no fieldset borders by default, and add a class when needed.
From #5, when do we need to hide borders on table headers?
Comment #8
falcon03 CreditAttribution: falcon03 commentedre#7: For instance, we would like to add table headings to the status report. See this issue for more details:
#1811128: No table header on install requirements
Comment #9
mgiffordSo with this we'd be basically stating that fieldsets in Drupal are semantic groupings. If you want a visual group, then use a detail.
This works for me. So should it go in core/modules/system/system.base.css
Comment #10
ry5n CreditAttribution: ry5n commented@mgifford On the contrary, both fieldsets and details have specific semantics and behavior (details are collapsibles). I think the idea here is that if you want a visual grouping, add a class. These kinds of repeated visual styles are ripe for abstraction; in the Seven style guide sandbox, we tentatively used a '.panel' class for fieldsets *and* details to apply border and padding. But we could also follow http://csswizardry.com/2011/10/the-island-object/
Comment #11
mgiffordYes, ry5n. I was being sloppy in my description. Fieldsets by default though in D8 would have no visual styling and would be largely used for grouping related elements like radios, checkboxes, dates & other elements which need them for accessibility. The approach suggested by the island object wouldn't help with the accessibility challenges we are trying to resolve.
Comment #12
ry5n CreditAttribution: ry5n commented@mgifford That was a bit of an aside about the island object. I understand the a11y reasons for this issue; +1. My point was to not use details for any visual effect, but I understand that’s not what you meant.
Comment #13
mgiffordTagging.
Comment #14
mgiffordI'm still a bit confused as to what we're looking for with the tables, but thought I'd roll up a patch to see bout addressing the fieldsets here in anycase.
I think that this is all stuff that should be part of system.theme.css. There is a problem making table headings invisible, but available to the screen reader.
This is just the CSS from https://drupal.org/files/cardinality-label-1932074.45.patch
Comment #15
mgifford#14: fieldset-css-2002336-14.patch queued for re-testing.
Comment #16
mgifford#14: fieldset-css-2002336-14.patch queued for re-testing.
Comment #17
mgifford#14: fieldset-css-2002336-14.patch queued for re-testing.
Comment #19
mgiffordThe core css definitions for fieldsets have moved to core/misc/drupal.base.css
I do think though that this patch probably should stay in core/modules/system/css/system.theme.css
I'm not clear though about why those elements have been moved over or why the drupal.base.css was created in the first place. Guess it has to do with conforming to the new coding standards here #1924430: Add drupal.base library for base CSS styles
Comment #20
perandre CreditAttribution: perandre commentedThe patch applies nicely, and the CSS would indeed remove borders from fieldset tag.
But: If this code is also intended for tables (and future UI elements), wouldn't it be better to just have a classed named "no-border"? The no-border css would still take presedence over the base styles anyways, and we get less bloat.
Comment #21
bserem CreditAttribution: bserem commentedchanging to RTBC
I believe Tables and future UI elements is better to be discussed in a new issue.
Comment #22
mgifford@perandre I'm not opposed to generalizing this, but in our testing with accessibility & tables we'd need to take a bit more invasive approach. I'm not sure that it could be generalized in a way that is also accessible.
I'd like to see tables discussed in a new issue. Say #2099761: Introduce a class to hide borders for tables
Comment #23
webchickJust asking (feel free to say no and set back to RTBC), is it possible to also convert one of the core fieldsets (such as the installer one) to using this class so we can confirm that it does what it says?
Comment #24
mgifford@webchick just to clarify, I can't recall what we'd change with the fieldsets in the installer. I'm not sure if you're asking so we can get a screenshot or if you want to commit this with an example. In either case, there's a related patch #504962: Provide a compound form element with accessible labels would probably be better.
Can we get this CSS change in, and then focus on bringing in the example ASAP?
Comment #25
webchickI was asking for an example to go in *with* the CSS change because currently there's no way to evaluate whether or not these classes actually do what they say they're going to do. :) Or, barring that, a before/after screenshot would also be fine.
Comment #26
mgiffordOk, so here's a good example on STM. With the patch from #504962-258: Provide a compound form element with accessible labels .
admin/structure/views/settings
user/1/edit
This is with the fieldset class applied. Without it, well, these 3 fieldsets just look like the boxy fieldsets we got rid of when we moved to details. Essentially.
Ok, I'll include the before shots, but just not include them as they are.. Well, not nice..
Comment #27
mgifford#19: fieldset-css-2002336-19.patch queued for re-testing.
Comment #28
mgifford@webchick - The images & use case were asked for and provided so putting this back to RTBC. Hope that's ok.
Comment #29
Xano19: fieldset-css-2002336-19.patch queued for re-testing.
Comment #30
webchickAwesome, thanks!
Committed and pushed to 8.x.
I'm thinking we might want a change notice for this? Including when module authors should use it.
Comment #31
LewisNyman CreditAttribution: LewisNyman commentedHi, sorry I missed reviewing this. The class that this selector has introduced is a conflict with our CSS standards.
As it happens, I've opened an issue to introduce a generic 'fieldset like' styling class for when module developers want to group items together. #2113903: Introduce a 'pane' component to visually group form items
I would suggest we remove the fieldset styling completely from system, and leave it up to a theme to define this styling.
Comment #32
mgifford@LewisNyman We need a way as part of Core to add fieldsets semantically to improve accessibility without affecting the page visually.
I think browsers apply fieldset styling by default which is against the look/feel of Core themes.
I'm not sure what the process is when Drupal's CSS standards conflict with implementing the WCAG 2.0 standards.
Comment #33
webchickOk, sounds like this still needs some more discussion so, rolled this patch back for now.
Comment #34
LewisNyman CreditAttribution: LewisNyman commented@mgifford I don't think it's that dramatic, I don't think there is a direct conflict between accessibility standards and our CSS standards. The solution that was committed did conflict with our standards.
If we have to add a class to undo styling, that's a very good clue that our styling is too broad. The correct way to handle this is to remove the default styling completely, and introduce a class to add the styling.
Here's a proposal in patch form, it might be more clear.
Comment #35
tstoecklerThe new approach makes a lot of sense, in my opinion. Is there any reason the class should be limited to forms, though?
Comment #36
mgiffordI tested this with SimplyTest.me and this approach works fine! This applies the latest patch from @LewisNyman above with this introduction of fieldsets for multi-part forms..
The secreenshot from /node#overlay=user/1/edit
Comment #37
webchickCool, this works for me. I expected this to remove border from everywhere (e.g. on pages like admin/config/development/performance) but it didn't seem to do that, so cool.
#35 raised a decent point, though, which wasn't really addressed. By naming the class "form-group" it implies that it can only be used on forms when in reality, it could potentially be used wherever.
This issue has been dragging on awhile, so let's timebox the discussion to a week (to allow for the holiday in the US). If we can't come up with anything better by then, we'll go ahead with this.
Comment #38
LewisNyman CreditAttribution: LewisNyman commentedIt should be, but there are very few places in core that actually use fieldset tags after #1168246: Freedom For Fieldsets! Long Live The DETAILS.
If we created a class like
.form-group
that added the same visual spacing, we wouldn't have to style details elements directly either and just apply this class.This is the same reasoning behind #2113903: Introduce a 'pane' component to visually group form items. Maybe we should take lead from the style guide and implement the same class in system. I know pane is a loaded term but so is module and we're using that in CSS.
Comment #39
mgifford34: fieldset-css-2002336-34.patch queued for re-testing.
Comment #40
mgiffordNot sure why these tags got removed. Wasn't intentional.
Comment #41
LewisNyman CreditAttribution: LewisNyman commentedRenaming to new objective. Hopefully it will be more clear.
Comment #42
mgiffordIt's been a week, so bumping it back to RTBC.
Comment #43
LewisNyman CreditAttribution: LewisNyman commentedBased on the suggestion in #35 I changed the class name to
pane
to match #2113903: Introduce a 'pane' component to visually group form items. This won't conflict with Panels because it uses the classpanels-pane
.I also found a few more places in core where we were using fieldsets that would need the class.
Comment #44
Jeff Burnz CreditAttribution: Jeff Burnz commentedWhy reset and add back, that affects all fieldsets, we should only normalize not reset. I don't see anything in the CSS standards that says we can't override browser default styles (which is actually what this still does).
To my mind this adds several layers of CSS wtfs and breaks CSS standards by using an overly generic class name and will highly likely false positive against something (like an external library). It also lacks any sort of real semantics, where I styled fieldset in Bartik I know what that is implicitly, now I have "pane", well, what is that?
Look, I get where you are coming from, you are saying fieldset is most likely to be used to wrap form elements as some sort of compound element, right, therefor remove the border. I would say that is an assumption and that core should not assume the OP as default, aka that groups of form elements should not have a border etc. That is a style and core should not really be in the business of styling my theme for me (at least as little as possible).
Comment #45
LewisNyman CreditAttribution: LewisNyman commentedHey Jeff :)
The points you've raised is something I've been trying to figure out as well. Adding a normalize to strip it out seems weird but what can we do when we are importing the normalize as a library? We can't modify it.
The point that this issue raises is that visual styling should be separate from this semantic meaning, because there are situations where you want one and not the other. See #2113903: Introduce a 'pane' component to visually group form items as well. This seems to be in line with our CSS standards.
Comment #46
Jeff Burnz CreditAttribution: Jeff Burnz commentedHey Lewis, long time no see and I apologise for that, I was away for more than 2 years due to ill health.
Seven or system.admin.css I think is more appropriate, in fact its was the system.theme placement which raised my eyebrows the most.
When I question the semantics I do so merely on the level of human comprehension, having a class for separation is fine, it's just the name of it. Generically named classes are kind of dangerous though, they do false positive and often badly because of external js libs that hook it somehow.
I'm not sure of what value it has outside of forms/admin, if it did then yes we're looking at something like a "pane object" and that has value, I'm just not sure what that value is.
Comment #47
LewisNyman CreditAttribution: LewisNyman commentedNice to have you back Jeff. I'm on Skype and IRC most weekdays if you feel like a chat.
If @mgifford is happy with moving this into admin CSS then let's move in this direction.
The reason we decided to expand the scope of #2113903: Introduce a 'pane' component to visually group form items beyond grouping form items is that the use cases we looked at where you would want to group items visually were consistent with the fieldset use case. These all perform the same purpose but the ones that are implemented are slightly different and consistent.
Examples:
Most exposed filters on views listing pages.
The navigation on the config page.
Details components
The sidebar thingy on the content creation page and block layout page
Comment #48
mgiffordFor most instances putting it in system.admin.css will probably be fine as the most common instances are in Seven's admin theme. However, there are going to be a number of places where someone would have radios/checkboxes or other multi-part forms in something other than the admin side. The Advanced search is certainly a big one as listed in fieldset-css-2002336-34.patch... This pattern would probably need to be repeated in a D8 version of Webforms for instance.
I just want this to be a common pattern and don't want to have to replicate it in other themes if we want to leverage that pattern. What's the concern with:
core/modules/system/css/system.theme.css
Other than that you don't want anything loading on the public pages if it isn't needed.
Comment #49
Jeff Burnz CreditAttribution: Jeff Burnz commentedThe point is Core should not be applying styles to html elements globally by default, the reset is a global style applied to all fieldsets, whether I want it or not. That's the sort of thing I as a themer do not want.
I think "no reset" is an entirely fair expectation for themers. I can live with normalize, but stripping off padding, margin and border is a very aggressive reset in any themers book.
In no way does this preclude your ability to have a reusable style for fieldsets, i.e. load the styles into .pane or what have you.
Look, Lewis was right to say that fieldset-no-border was a violation, it was, but the alternative proposed is worse. However, the basic underlying idea to load up an object like class that is reusable is good, so do it. Just don't kill kittens while your at it, aka reset elements to save a few lines of CSS, it impacts on all forever after.
Comment #50
LewisNyman CreditAttribution: LewisNyman commentedOk, here's a patch that only applies the change to admin facing styles.
Comment #52
mgifford50: fieldset-css-2002336-50.patch queued for re-testing.
Comment #53
mgiffordOk, so based on this:
I'm guessing we can't insert this attribute here:
'#attributes' => array('class' => array('pane')),
.Comment #54
LewisNyman CreditAttribution: LewisNyman commentedComment #55
LewisNyman CreditAttribution: LewisNyman commentedI found a page that still uses a fieldset!
/admin/config/media/image-toolkit
Comment #56
mgifford50: fieldset-css-2002336-50.patch queued for re-testing.
Comment #57
LewisNyman CreditAttribution: LewisNyman commentedI removed the class from the node search forms because they are front facing forms anyway. I've added the class to the image styles form.
Comment #58
philipz CreditAttribution: philipz commentedI've applied patch #57 and I'm looking on theme settings and performance pages.
There are no elements with class '.pane' and therefore the patch is not styling anything. I wonder if something else changed and patch needs re-roll to adapt the changes or I'm missing something.
Comment #59
mgifford57: fieldset-css-2002336-57.patch queued for re-testing.
Comment #61
LewisNyman CreditAttribution: LewisNyman commentedReroll.
I'm struggling to find anywhere in core where we actually use fieldsets now? It looks like they've all bee converted to details elements...
Comment #62
mgiffordIt's easy to find if you apply the patch from #504962: Provide a compound form element with accessible labels
Most of them were taken out, but I know that there are one or two left. I'm not sure exactly where though, sorry.
Most of them were converted to details.
Comment #63
Jeff Burnz CreditAttribution: Jeff Burnz commentedLooks good to me.
FYI I use fieldset a lot still (in D8), and this actually helps me out a heap, so great!
Comment #64
webchickI found a spot, which is Views UI wizard. I've no idea why/how, but this patch seems to make it very inconsistent. (Firefox 26.0, Mac OSX)
Before:
After:
Comment #65
mgiffordThe last two are inconsistent as they are missing the CSS class "pane".
How are they generated in Views?
Comment #66
mgiffordAhh.. Wizards can really suck some times. Because we hadn't edited core/modules/views/lib/Drupal/views/Plugin/views/wizard/WizardPluginBase.php
It would have failed on Page settings, Page display settings, Block settings & Block display setting
Comment #67
mgiffordWith the latest patch.
Comment #68
sunHrm. Sorry, coming late to this party.
I vehemently disagree with the proposal to apply a default styling to fieldsets that no longer makes them appear as regular fieldsets.
The reasoning is simple:
The quoted part of our CSS standards in #31 was applied to the wrong context — only custom styles should not have to override other custom styles to revert their styling.
But this is about browser default styling. If I output a
<fieldset>
, then I expect to get a standard fieldset. With default borders, padding, and the legend in the top border.Overriding the default styling of a standard HTML element for a particular and custom use-case in the system's default CSS is the exact opposite of what default styles should do. → If you have a specific use-case for styling a standard HTML element in a custom way, then that's what classes are for.
Therefore, the originally proposed patch that used
fieldset.fieldset-no-border
was correct.But that said, the original patch used a too specific selector that duplicated itself; it should either be
fieldset.no-border
or just.fieldset-no-border
.Additionally, "no-border" does not fully encompass the effect, because the default padding is removed, too. → .no-box or .plain or .flat or similar would be more accurate class names.
Comment #69
mgiffordI don't care one way or the other about how this box is over-ridden, as long as there is a way to do it.
As usual, @sun makes a good argument, so I've included the patch.
The previous patch addressed border, margin & padding by making them all 0. Not sure if that's a better approach or not:
Comment #70
LewisNyman CreditAttribution: LewisNyman commentedThanks Sun.
I'm not sure it is. This is about how admin themes handle fieldset styling. I don't think should be any expectation of style from an html element alone, unless you're developing a theme, which we addressed in #44.
We also have to consider the use case of wanting the style without the element. See #47 That's a strong argument for separating styling from the element. Our CSS standards are built around the goal of reusable classes.
Maybe this should just sit within the Seven theme? I think that would give contrib admin themes a bit of a tricky situation to deal with.
Comment #71
mgifford@LewisNyman, there has to be something in core (not just Seven) that allows us to deal with #504962: Provide a compound form element with accessible labels
Ultimately this is a very small amount of CSS we're considering here. It may not fit perfectly within the guidelines, but I do know that it will allow an important accessibility issue to be brought into Core.
Comment #72
sunThanks!
The diff context of this patch reveals that we have another CSS class
.fieldset-no-legend
already, so for the sake of consistency, I'd recommend to use.fieldset-no-box
here for now.→ Let's consider to open a follow-up issue to remove the
fieldset-
prefix from both afterwards.Regarding the fieldset no-box styles, I'd (1) restrict the border to just border-width and (2) margin most likely should be left alone, because fieldsets typically appear in between of .form-items as well as after other fieldsets, and we want to retain some sane vertical spacing. Thus:
Regarding the legend no-box styles, I don't see why those are contained in system.theme.css. Only the Seven theme applies a text-transform:uppercase to fieldset legends, so at minimum, that style override needs to be moved into Seven's CSS.
Additionally, the selector should only target the immediate child, because fieldsets can contain fieldsets:
That said, I'm not sure I understand why the uppercase styling is reverted here? I can only guess that we're trying to cater for different use-cases:
For B), this override should actually not happen, because fieldsets and details would appear differently in Seven otherwise.
For A), I assume that the selector for Seven's CSS should actually be more targeted, along the lines of this:
In other words, only revert the uppercasing if the fieldset is used as a wrapper for the compound radios/checkboxes form elements.
All of that being said, this adjustment has nothing to do with this issue here and belongs into #504962: Provide a compound form element with accessible labels instead, because that issue is the one that wants to use fieldsets in a new/additional way.
→ Let's remove the text-transform override styling entirely from this patch.
Comment #75
sun@LewisNyman:
That's the first misconception from my perspective, because fieldsets can appear in front-end themes, too. The context of an "admin theme" is irrelevant here.
By saying that, I can only assume that you're a fan of the former reset.css approach instead of the normalize.css approach. The former concept did exactly what you're saying: Force-remove all expectations that anyone could possibly have from all HTML elements. Conversely, the latter concept is no longer doing that, but instead: Harmonize the default styling of all HTML elements across browser vendors.
This means that (1) there are expectations for how a particular HTML element is rendered by browsers by default and (2) as a themer, I expect basic HTML elements to mostly appear with their default styling, give or take a few sane assumptions — but overriding the default style for a completely custom use-case is not at all a sane assumption.
@Jeff Burnz appears to have raised the exact same red flags before already.
That makes little sense, because the ultimate goal is to introduce this alternate styling for fieldsets as a base concept for Drupal's user interface. We'll end up with:
Comment #76
LewisNyman CreditAttribution: LewisNyman commentedIf this is the case then I agree with everything you say after because we definitely don't want to affect default styling on the front-end.
Reading over the requirements in #48 that looks right:
So this patch is the only way to solve this problem. RTBC.
Comment #77
sunSorry, I really don't want to hold this up any further (since this issue still blocks progress elsewhere), but I wanted to address the concern that @webchick originally raised in #25 already → include an example to prove that this works as intended.
In order to address that, I need to introduce a new #type in this issue already, because we don't want to add arbitrary CSS classes to individual form elements (which essentially makes #2113903: Introduce a 'pane' component to visually group form items obsolete).
I did a quick poll in IRC to see whether "pane" is really an accurate name for this user interface component. After a brief discussion, for now I decided to go with #type "fieldgroup" and renamed the CSS class accordingly.
Note that various people would prefer if #type 'container' would simply be adjusted to conditionally become an unboxed fieldset, in case a #title property has been set. — I'm not sure whether it is technically legit to use a
<fieldset>
element outside of a<form>
context, since the HTML5 spec for 'fieldset' and HTML language reference both contain very clear wording like this (emphasis mine):Since #type 'container' is definitely used outside of forms, conditionally turning it into a fieldset might not be valid (again, I'm not sure).
Because the only purpose for now is to have a proof for the concept, I think it would be best if we'd hash out the final incarnation of the element #type in a follow-up issue; possibly re-using/re-purposing the existing #2113903: Introduce a 'pane' component to visually group form items
All of that being said, here's the visual proof based on attached patch, which converts the details in the installer into #type fieldgroup:
Comment #78
barraponto CreditAttribution: barraponto commented@sun I guess those forms were already WCAG compliant (and were fine with a bordered fieldset). Instead, what the issue wants to address are other groupable form controls that shouldn't have visible fieldset styles.
The first example at the WCAG technique page seems to be exactly the case. http://www.w3.org/TR/2008/WD-WCAG20-TECHS-20080430/H82.html
Comment #79
mgifford@sun the main reason for this patch as far as I'm concerned is so that we can introduce fieldsets in #504962: Provide a compound form element with accessible labels without changing the UI.
This addresses an outstanding issue we've been trying to address since 2009. This is the biggest outstanding accessibility problem in D7's (and D8) forms.
That is what we should be testing this patch against rather than creating some other patch to test. And that's what I've been trying to do since @webchick's post in #25.
EDIT: @barraponto the issue with compound form elements is a great example of that WCAG technique you linked to.
Comment #80
barraponto CreditAttribution: barraponto commented@mgifford if I understood correctly, this issue is a blocker for the compound form elements issue, right?
Comment #81
sunTo address the remarks that came up: (not sure whether the recent comments were meant to raise concerns/problems on the actual patch?)
Yes. This issue has the unfortunate aspect of a dual-purpose attached to it:
The cleverly designed CSS class for fieldsets that is being introduced here not only presents a dependency for #504962: Provide a compound form element with accessible labels — it also happens to present a base building block for a new + primary user interface component that is long overdue for Drupal core to offer:
A fieldset whose legend just simply appears similar to a heading, which is not "collapsible", but yet, which still groups child form elements into a semantically correct container.
Since the day we replaced our custom notion of collapsible fieldsets with HTML5 standards-compliant Details, Drupal's user interface is littered with Details elements that by design, should (literally) contain "details" only — because it is the one and only user interface component that presents a grouping mechanism in core.
By introducing the CSS component, we're inherently enabling the badly needed user interface component, because the UI component does not need anything else aside from the CSS class that is being introduced here.
The concrete example given in #77 only underlines that: It replaces the Details elements of the last page in the Drupal installer with the new UI component for grouping form elements, because none of these are "details"; they present the primary interaction point for the end-user and each set of the fields has to be processed by the user.
Compound form elements of radios and checkboxes are to be grouped (invisibly). But form elements contained in a (visible) group need to be grouped for accessibility, too.
→ The proposed solution not only unblocks #504962: Provide a compound form element with accessible labels , it additionally ensures that WCAG requirements are met for intentionally visible groups of form elements in Drupal's user interface.
In fact, given the design of this implementation here, it might be possible that #504962 can be resolved in a very different way. I'll explore that once this issue is done.
Everyone of us is actually shooting for the exact same goal: Moving forward. :-)
As already mentioned in #77, the exact and final name of the new element #type + possibly also its technical implementation should be re-evaluated and can happily be refactored in a follow-up issue. The whole point was and is to address a fundamental concern that has been raised at the beginning of this issue: Supply an actual proof to assert that the new CSS component actually fulfills its job. :)
Comment #82
mgifford@barraponto - Yes. As I understand it, this is a blocker.
@sun - Thanks for detailed responses.
1. I now understand better the example in #77 where fieldsets are a more appropriate pattern than details.
2. Agreed.
3. Good point.
I thought I had provided that proof back in #36 where I had combined two patches using SimplyTest.me. I'm happy for it to be illustrated a different way as well.
Comment #83
barraponto CreditAttribution: barraponto commentedThe implementation provided by @sun is enough for case 1 (which is basically #504962: Provide a compound form element with accessible labels ). But for case 2, the WCAG 2.0 compliant fieldgroups (which is the subject of this issue), we still need to work on a visual proof.
I think we can add a new
#theme_wrapper
to checkboxes and radios element definitions to add the fieldgroups. Or maybe wrap it in a proper fieldgroup element duringform_process_radios
andform_process_checkboxes
.I tried the latter and failed.
Comment #84
mgiffordThis SimplyTest.me link combines the patch (similar to the previous one).
Attached are screenshots. The duplication of the fieldgroup class is an issue (caused by the two patches). Also the width of the text inputs seems off.
Comment #85
sunI don't quite understand why the provided proof of replacing the details on the "Configure site" installer page is not sufficient?
Btw, quoting myself from #81.2:
I preemptively explored that and it works excellently. :-) — In order to not hi-jack the original issue/thread with my alternative proposal, I supplied a working patch in a new sister/competing issue: #2192419: Use a WCAG-compliant fieldset (fieldgroup) for #type radios/checkboxes
As a result, I still think (1) the latest patch here is RTBC and (2) we can happily have a follow-up issue to re-evaluate/discuss the name and technical implementation of the element #type.
Comment #86
mgiffordI think there's still a spacing issue in the example.
I think I was unsure where you were going with the other code. If we can get the formatting cleaned up I think we can mark this RTBC.
Comment #87
sunThanks for discovering that — I did not notice that gem, since my browser window was wide enough.
I spent quite some time with debugging the CSS, studying the specs and common references, as well as researching potential workarounds. It appears to be a known and unresolved problem with input elements and the only working solution seems to be this:
#2193271: Remove default #size attribute from core
Comment #88
sunComment #89
mgiffordSo can we mark this RTBC before we remove the #size attribute?
Or do we just need to set the css width to have a minimum width of 90% of it's container?
Comment #90
sunTechnically yes, we should be able to move forward, as these are two independent issues — the 'size' attribute problem exists in HEAD already; it's just less apparent.
Comment #91
mgiffordIn that case... I've looked over the code, applied it and tested it with a similar use case.
Thanks @sun!
Comment #92
catchCommitted/pushed to 8.x, thanks!
Comment #93
longwaveAs noted in #86, this patch broke the layout on the "Configure site" page of the installer, this has already been reported in the following issues:
#2195781: Configure Site page on install is badly formatted
#2197187: Install output error
Comment #95
karimiehsan1819 CreditAttribution: karimiehsan1819 commentedhi , i want hide or remove label and border of field collection , how can i do them?
Comment #96
Liam MorlandPlease open a new support issue for your question.