fix(Forms): align Value.SummaryList when Value.* has no label #4311
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| <Dt className="dnb-forms-value-block__label"> | ||
| {label && <strong>{label}</strong>} | ||
| </Dt> | ||
| {label && ( |
There was a problem hiding this comment.
I think this will work, since a Dl must not have a Dt and Dd I believe, only one or more of one of them.
There was a problem hiding this comment.
A dd needs a dt. It's the term of the description. Like a label for an input.
We should not allow it. We should actually "warn" devs when they not provide a label. But it could potentially be a visually hidden label (srOnly is not supported today).
But will it ever make sense?
There was a problem hiding this comment.
Not sure. But if it's required to have a label, then the property should not be optional as it is today.
There was a problem hiding this comment.
I'd like to share the code for the example I provided.
Basically the heading of the section works as a label for the name, and the name is part of a SummaryList for easy alignment.
This can be structured differently.
So if the answer is "won't fix", then we'll just do it a different way.
And I agree with @langz that it shouldn't be optional in that case 👍
<Form.Section.ViewContainer
title="Hovedkontaktperson"
variant="basic"
>
<Value.SummaryList layout="horizontal">
<Value.Composition>
<Value.String path="/firstName" showEmpty />
<Value.String path="/lastName" showEmpty />
</Value.Composition>
<Value.PhoneNumber
label={MainContactPerson.phoneNumber}
path="/phoneNumber"
showEmpty
/>
<Value.String
label={MainContactPerson.emailAddress}
path="/emailAddress"
showEmpty
/>
</Value.SummaryList>
</Form.Section.ViewContainer>
There was a problem hiding this comment.
I will add a labelSrOnly property so you can provide a label that is not visible (in another PR). But alternatively, you can move Value.Composition out of the Value.SummaryList I think.
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
langz
force-pushed
the
fix/align-value-without-label-in-horizontal-summarylist
branch
from
666356f to
5cb734d
Compare
| @@ -162,6 +162,10 @@ | |||
| margin-top: 0; | |||
| margin-right: calc(var(--column-gap) - 0.5rem); | |||
| max-width: var(--dt-max-width); | |||
|
|
|||
| &:empty { | |||
| display: contents; | |||
There was a problem hiding this comment.
Not 100% sure if display: contents; is the best.
Other alternative could be display: none?
When trying to just set margin-top: 0; to counter the margin-right: calc(var(--column-gap) - 0.5rem);, that resulted in too big of a spacing/gap above description 2 and the line above, see the following image:
Any thoughts @tujoworker? 🤔
There was a problem hiding this comment.
display changes the way a screen reader experience the content. Sure, its "empty". But I think we rather should fix the visual issue, than chaining the semantics.
This here should work:
.dnb-dl__layout--horizontal dt:empty {
margin-right: 0;
}
There was a problem hiding this comment.
Isn't it what I tried, and reported did not work? 🤔
I can change from display: contents; to margin-right: 0;, but will expect that it still do not work.
Take a look at the commit 4a7c8cb
Screenshot tests/visual tests doesn't look correct too me.
Anyways, I'm in the car now, and can't make any changes. So feel free to make changes if you'd like, so that we can release 🚗
There was a problem hiding this comment.
Right, you don't warn it to take space in the vertical.
I'll add a class dnb-sr-only – it would ensure the height is 0.
langz
changed the title
tujoworker
changed the title
tujoworker
force-pushed
the
fix/align-value-without-label-in-horizontal-summarylist
branch
from
d606d8a to
207088a
Compare
tujoworker
force-pushed
the
fix/align-value-without-label-in-horizontal-summarylist
branch
from
207088a to
0f564d8
Compare
langz
deleted the
fix/align-value-without-label-in-horizontal-summarylist
branch
Based on #4311 --------- Co-authored-by: -l <anderslangseth@gmail.com>
## [10.57.0](v10.56.0...v10.57.0) (2024-11-22) ### 📝 Documentation * **Field.Upload:** adds `asyncFileHandler` property ([#4288](#4288)) ([fb09758](fb09758)) * **Field.Upload:** adds asyncFileHandler property ([7ccdabd](7ccdabd)) ### 🐛 Bug Fixes * **Forms:** align Value.SummaryList when Value.* has no label ([#4311](#4311)) ([b6621c2](b6621c2)) * **Forms:** ensure fields inside composition share submit indicator ([#4309](#4309)) ([e726e20](e726e20)) * **Forms:** safeguard errorMessages to avoid infinite loops when not wrapped in useMemo ([#4305](#4305)) ([f14bacc](f14bacc)) * **Forms:** show indicator with async onBlurValidator call when `validateInitially` is used ([#4303](#4303)) ([c585491](c585491)) * **Icon:** ensure icon name is rendered as `data-testid` ([#4304](#4304)) ([235b823](235b823)) ### ✨ Features * **Card, Section:** add `outset` property for moderate layout breakout ([#4317](#4317)) ([6008d9a](6008d9a)) * **DrawerList, Dropdown, Autocomplete, Field.Selection, Field.ArraySelection:** disabled options ([#4154](#4154)) ([8786d5d](8786d5d)) * **Field.Upload:** adds support for async and sync fn in `fileHandler` ([#4294](#4294)) ([2cc816a](2cc816a)) * **Forms:** add `Form.Card` with different default appearance than Card (use `Form.Card` in forms instead of Card) ([#4318](#4318)) ([7bbc0ca](7bbc0ca)) * **Forms:** add `labelSrOnly` to Value.* components ([#4319](#4319)) ([46f68ae](46f68ae)), closes [#4311](#4311) * **Forms:** deprecate `validator` in favor of `onChangeValidator` ([#4314](#4314)) ([5a06b2e](5a06b2e)) * **Typography:** add general `.dnb-t` helper classes and add typography props to Span ([#4235](#4235)) ([9dfe66d](9dfe66d))
|
🎉 This PR is included in version 10.57.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #3793