Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug(Forms): Different amount of spacing between Field components. #3102

Closed
langz opened this issue Dec 16, 2023 · 6 comments · Fixed by #3187
Closed

bug(Forms): Different amount of spacing between Field components. #3102

langz opened this issue Dec 16, 2023 · 6 comments · Fixed by #3187
Assignees
Labels
bug Something isn't working released
Milestone

Comments

@langz
Copy link
Contributor

langz commented Dec 16, 2023

To me, when browsing the docs on iPhone in Safari, it seems like there’s different spacing between the Field components in a card demo/example here https://eufemia.dnb.no/uilib/layout/#flex-usage-in-forms

IMG_4967

To me, it seems like the spacing is different between the fields e-post and landskode, compared to spacing between all other fields.

@langz langz added the bug Something isn't working label Dec 16, 2023
@langz langz added this to the Forms Release milestone Dec 16, 2023
@tariknh
Copy link

tariknh commented Dec 17, 2023

Could this be because landskode is correlated to the mobilnummer input, as in the law of proximity ? https://lawsofux.com/law-of-proximity/

@snorrekim
Copy link
Contributor

snorrekim commented Jan 8, 2024

This just seems to be a media query rule for the all our basic input components. But it only works if the <label> is below the base class for the component, and in <Forms> regular inputs with the class .dnb-input have the <label> before the base class. So the margin is only added to widgets like .dnb-autocomplete, .dnb-date-picker etc.

Expected structure:

<span class="dnb-autocomplete">
  <label>Label</label>
  <span class="dnb-autocomplete__inner">
    ...
  </span>
</span>

<span class="dnb-input">
  <label>Label</label>
  <span class="dnb-input__inner">
    ...
  </span>
</span>

<Form> structure:

<span class="dnb-autocomplete">
  <label>Label</label>
  <span class="dnb-autocomplete__inner">
    ...
  </span>
</span>

<div class="dnb-forms-field-block">
  <div class="dnb-forms-field-block__grid">
    <label>Label</label>
    <div class="dnb-forms-field-block__contents">
      <span class="dnb-input">
        <span class="dnb-input__inner">
          ...
        </span>
      </span>
    </div>
  </div>
</div>

@snorrekim
Copy link
Contributor

To put it on other words: If the label is set on the <FieldBlock> component instead of the input component, then the media query will not work. This happens in the <StringComponent> and <NumberComponent> (maybe others?).

@snorrekim snorrekim self-assigned this Jan 8, 2024
@snorrekim
Copy link
Contributor

But the question is, do we want that extra margin on small screens or not?

@tariknh
Copy link

tariknh commented Jan 8, 2024

I think it’s supposed to happen, as a "block" has children/siblings that is correlated, therefore less margin

snorrekim added a commit that referenced this issue Jan 8, 2024
…#3187)

Fixes #3102

FieldBlock label now has the same top-margin on small screens as other input fields.
tujoworker pushed a commit that referenced this issue Jan 8, 2024
…#3187)

Fixes #3102

FieldBlock label now has the same top-margin on small screens as other input fields.
tujoworker pushed a commit that referenced this issue Jan 11, 2024
## [10.17.0](v10.16.0...v10.17.0) (2024-01-11)

### 📝 Documentation

* refactor custom field component docs and example ([#3193](#3193)) ([3871079](3871079))

### ✨ Features

* **Anchor:** blank target icon styling  ([#3172](#3172)) ([2ca216a](2ca216a))
* **DatePicker:** add onFocus event ([#3188](#3188)) ([2062213](2062213))
* **Field.Number:** add step control functionality ([#3140](#3140)) ([a9e1697](a9e1697))
* **Form.Visibility:** add `pathValue` and `whenValue` ([#3206](#3206)) ([8a547ae](8a547ae))

### 🐛 Bug Fixes

* **Autocomplete:** only set aria-controls attribute when expanded ([#3180](#3180)) ([a16a7ba](a16a7ba))
* **DatePicker:** add return object for onBlur ([#3178](#3178)) ([18b3889](18b3889))
* **DatePicker:** fix bug where minDate set to today is disabled ([#3176](#3176)) ([ef65676](ef65676))
* **Dropdown:** correct aria-controls target element ([#3181](#3181)) ([bc26101](bc26101))
* **Field.Currency:** handle big numbers ([#3184](#3184)) ([b856a46](b856a46)), closes [#3124](#3124) [#3185](#3185)
* **Field.Number:** replace `rightAligned` prop with `align` ([#3175](#3175)) ([ba130ba](ba130ba))
* **FieldBlock:** ensure extra spacing of labels when on small screens ([#3187](#3187)) ([c86c857](c86c857)), closes [#3102](#3102)
* **FieldBlock:** label can be clicked after focusing input ([#3190](#3190)) ([95b37e7](95b37e7)), closes [#2979](#2979)
* **Flex.Container:** add `Flex.withChildren` HOC for handling wrapped children with spacing ([#3200](#3200)) ([93df77c](93df77c))
* **Flex.Container:** show line divider even when heading is present ([#3198](#3198)) ([d135b47](d135b47))
* **Form.Visibility:** move Visibility to Form.Visibility ([#3205](#3205)) ([d3b766f](d3b766f))
* **InputMasked:** correct cursor position when navigating using keyboard ([#3160](#3160)) ([9143b38](9143b38))
* **InputMasked:** extend reliability on cursor position correction ([#3194](#3194)) ([cbc27ac](cbc27ac)), closes [#3160](#3160)
* **MultiInputMask:** enhance handling of right, left and backspace key usage ([#3192](#3192)) ([f3ce934](f3ce934))
* **PhoneNumber:** show read outline on both fields in error state ([#3196](#3196)) ([16ed821](16ed821))
* **Textarea:** ensure correct height based on rows for Firefox browser ([#3207](#3207)) ([75f754e](75f754e))
* **Visibility:** add support for being used in Flex.Container ([#3203](#3203)) ([7a72fa6](7a72fa6))
@tujoworker
Copy link
Member

🎉 This issue has been resolved in version 10.17.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Development

Successfully merging a pull request may close this issue.

4 participants