Field: demo with horizontal layout is broken
Closed, ResolvedPublic3 Estimated Story PointsBUG REPORT
Actions

Assigned To

Authored By

	AnneT
	Mar 25 2024, 7:39 PM

Description

Steps to replicate the issue (include links if applicable):

Visit https://doc.wikimedia.org/codex/main/components/demos/field.html#fieldset-with-nested-fields

What happens?:

The horizontal layout is broken due to a margin-top on the second field:

What should have happened instead?:

The two fields should appear vertically aligned to the top:

Software version (skip for WMF-hosted wikis like Wikipedia): Codex 1.3.5

Solution

We need to determine how to solve this—is it an issue with the margin-top placed on fields in Codex, or an issue with this specific demo?

We do not explicitly support horizontal field layouts at this time. The new margin for fields only takes into account a vertical layout. If we want to continue this paradigm, we should simply add a style to the demo to remove the margin-top of the second field.

However, if we want to at least improve support for horizontal field layouts, which we do recommend in the field/form guidelines, we should reconsider the margin-top. Some other options:

Use a margin-bottom instead (removing it on the last field). This would make horizontal layouts appear less broken, but the margin-bottom would still be problematic in the case that the last field is longer (taller) than the ones next to it, yet it has no margin-bottom, so the spacing between the horizontal field layout and whatever's below it is too small. The demo in question is an example of this - if it had, say, another fieldset below it, the margin between the two fieldsets might appear too small, since the "Weight value" field's margin-bottom doesn't start below the "Unit" field's help text.
Set the vertical margin by default, but add an option to set a horizontal margin instead

I think a combination of these might be ideal. However, since we haven't determined exactly how to support horizontal field layouts in Codex, I recommend we just do the first one for now.

Acceptance criteria

Agree on a solution and document it in this task
Implement the solution

Details

	Subject	Repo	Branch	Lines +/-
	Update Codex from 1.3.6 to 1.4.0	mediawiki/core	master	+190 -114
	docs: fix nested Fields vertical alignment	design/codex	main	+6 -1

Customize query in gerrit

Event Timeline

AnneT created this task.Mar 25 2024, 7:39 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 25 2024, 7:39 PM

Tagging @lwatson, @Catrope, and @Volker_E since y'all have discussed this margin recently. I don't know exactly why it was implemented the way it was, so please feel free to update my opinionated summary in the task if you have more insight!

Thanks for filing this @AnneT - We discussed approaches to add spacing between Fields but didn't account for horizontal Field layouts. Margin-top was considered the best approach - but only for vertically stacked Fields.

Reedy added a project: Codex.Mar 25 2024, 10:18 PM

Restricted Application added a project: Design-System-Team. · View Herald TranscriptMar 25 2024, 10:18 PM

Following the new Constructing forms guidelines, the padding between elements within a module grouping multiple fields and/or actions should use the spacing-100 token (equivalent to 16px in the default Codex theme). So we should increase the padding between these 2 fields in the set:

cc: @DTorsani-WMF

@bmartinezcalvo Thanks for adding this spacing guideline. That is true. 16px is the spacing between fields within a fieldset, since they are more closely related, with 24px being the default spacing between fields. That being said, we may want to update this guidance for ease of implementation and a more consistent visual flow, since then some fields would be spaced slightly differently even when vertically stacked just because they're in a fieldset, which might look a bit unintentional.

CCiufo-WMF moved this task from Inbox to Up Next on the Design-System-Team board.Mar 26 2024, 1:44 PM

In case it's relevant here: the spacing between fields on the login form is currently 15px. I have a temporary override in place to keep that the same for now, but if we remove that and let Codex's default behavior take over, then the spacing between each of the fields on the login form would be 24px, which does look pretty spacious.

Thanks for testing this @Catrope. Are you specifically testing this spacing between horizontal fields or also vertically stacked fields?

That comment was about vertically stacked fields, which is why I wasn't sure it was relevant. To see the login form thing that I'm talking about, compare the current appearance of the login form (with the legacy 15px spacing) with this demo of what the login form would look like if we removed the legacy styles and allowed Codex's default field styles (with 24px spacing) to apply. I brought it up mostly because, if there's any discussion of reevaluating the 24px spacing and perhaps changing it to 16px, then that might affect what we do with the login form and how carefully we roll it out (16px is so close to the existing 15px that we wouldn't expect anyone to notice, whereas 24px would be a visible change to a prominent form).


Current appearance (15px spacing)	Codexified appearance (24px spacing)

That's a great point. Thanks for demonstrating this. I think changing this to be 16px between fields is the right solution. We will need to update the forms guidelines on the docs site but I can take care of that.

Here is the task for updating this which I will work on.

Catrope set the point value for this task to 3.Mon, Apr 1, 5:26 PM

Catrope moved this task from Up Next to DST-Sprint-20 (2024-04-01 to 2024-04-12) on the Design-System-Team board.

Catrope edited projects, added Design-System-Team (DST-Sprint-20 (2024-04-01 to 2024-04-12)); removed Design-System-Team.

Volker_E claimed this task.Tue, Apr 2, 5:39 PM

Change #1017184 had a related patch set uploaded (by VolkerE; author: VolkerE):

[design/codex@main] docs: fix nested Fields vertical alignment

https://gerrit.wikimedia.org/r/1017184

gerritbot added a project: Patch-For-Review.Fri, Apr 5, 4:14 AM

Volker_E changed the task status from Open to In Progress.Fri, Apr 5, 4:15 AM

Volker_E moved this task from Committed to Code Review on the Design-System-Team (DST-Sprint-20 (2024-04-01 to 2024-04-12)) board.

Catrope moved this task from Code Review to Pending Release on the Design-System-Team (DST-Sprint-20 (2024-04-01 to 2024-04-12)) board.Mon, Apr 8, 4:34 PM