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

feat(DatePicker): add size property to component #1438

Merged
merged 1 commit into from Jun 10, 2022

Conversation

langz
Copy link
Contributor

@langz langz commented Jun 8, 2022

This PR adds the size property to the DatePicker component, similarly as for components like Input, etc.

image

@langz langz linked an issue Jun 8, 2022 that may be closed by this pull request
@langz langz force-pushed the feat/add-size-prop-to-date-picker branch from 3a0d2d6 to ee17eb3 Compare June 8, 2022 20:43
@@ -374,6 +402,34 @@
border: 1px solid var(--color-black-border);
background-color: var(--color-white);
}

This comment was marked as resolved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice work so far @langz 🙌

Two things I suggest:

  • We should not change the size of the triangle.
  • We should probably not show the other sizes in the documentation, but show them for visual tests. And with that, we could keep them in only one visual snapshot test. The reason is, in future we only want to set the size in a spacing "Theme" that changes the component sizes. This way, we can avoid wrong usage of components. But also align better on overall consistency.

I hope you are fine with my arguments so far. Let me know if you disagree or have any questions.

Copy link
Contributor Author

@langz langz Jun 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds smart, I'm fine with that 😄
I'll look into it sometime today 🕐

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've addressed these things now 😺

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 8, 2022

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.

Latest deployment of this branch, based on commit 31463e9:

Sandbox Source
eufemia-starter Configuration

@gatsby-cloud
Copy link

gatsby-cloud bot commented Jun 8, 2022

Gatsby Cloud Build Report

DNB Eufemia Portal

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 10m

Performance

Lighthouse report

Metric Score
Performance 🔶 64
Accessibility 💚 100
Best Practices 💚 100
SEO 💚 92

🔗 View full report

@langz langz force-pushed the feat/add-size-prop-to-date-picker branch 3 times, most recently from ddf19a5 to 710b95c Compare June 9, 2022 10:40
@langz langz requested a review from tujoworker June 9, 2022 11:14
@@ -310,12 +312,12 @@ export default class DatePicker extends React.PureComponent {
.querySelector('.dnb-input__submit-button__button')
.getBoundingClientRect().width
if (align_picker === 'right') {
const distance = buttonWidth / 2 - 8

This comment was marked as resolved.

@langz langz requested a review from OddKMS June 9, 2022 11:41
Copy link
Contributor

@OddKMS OddKMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like what I see, love the tests.

I'm putting an approve as soon as you've implemented the changes suggested by Tobias :)

@langz langz force-pushed the feat/add-size-prop-to-date-picker branch 2 times, most recently from db1c480 to 02f0993 Compare June 10, 2022 08:55
@langz langz requested a review from OddKMS June 10, 2022 09:00
Copy link
Member

@tujoworker tujoworker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YesYesYes – very very nice work 👌

@tujoworker tujoworker force-pushed the feat/add-size-prop-to-date-picker branch from 02f0993 to 66c0049 Compare June 10, 2022 10:09
@tujoworker
Copy link
Member

I rebased this branch with the latest from main – hope it helps for the continues failing visual tests. Not sure why they have failed randommely right in this branch. I feel they are else pretty stable.

@langz langz force-pushed the feat/add-size-prop-to-date-picker branch from 66c0049 to 40db891 Compare June 10, 2022 11:30
@langz
Copy link
Contributor Author

langz commented Jun 10, 2022

I rebased this branch with the latest from main – hope it helps for the continues failing visual tests. Not sure why they have failed randommely right in this branch. I feel they are else pretty stable.

Yeah, it somehow seems to always timeout when running tests for Input.screenshot.test.js, both locally and in build server. Tried running main branch locally as well, and it fails there too 🤔

image

In text:

 FAIL  src/components/input/__tests__/Input.screenshot.test.js (361.671 s)
  ● Input screenshot › have to match input with placeholder

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      19 |   setupPageScreenshot({ url: '/uilib/components/input/demos' })
      20 |
    > 21 |   it('have to match input with placeholder', async () => {
         |   ^
      22 |     const screenshot = await testPageScreenshot({
      23 |       ...extend('input-placeholder'),
      24 |       selector: '[data-visual-test="input-placeholder"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:21:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match input with icon

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      27 |   })
      28 |
    > 29 |   it('have to match input with icon', async () => {
         |   ^
      30 |     const screenshot = await testPageScreenshot({
      31 |       ...extend('input-icon'),
      32 |       selector: '[data-visual-test="input-icon"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:29:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match disabled input

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      35 |   })
      36 |
    > 37 |   it('have to match disabled input', async () => {
         |   ^
      38 |     const screenshot = await testPageScreenshot({
      39 |       ...extend('input-disabled'),
      40 |       selector: '[data-visual-test="input-disabled"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:37:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match search type

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      43 |   })
      44 |
    > 45 |   it('have to match search type', async () => {
         |   ^
      46 |     const screenshot = await testPageScreenshot({
      47 |       ...extend('input-search'),
      48 |       selector: '[data-visual-test="input-search"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:45:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match search type with focus state

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      51 |   })
      52 |
    > 53 |   it('have to match search type with focus state', async () => {
         |   ^
      54 |     const screenshot = await testPageScreenshot({
      55 |       ...extend('input-search'),
      56 |       selector: '[data-visual-test="input-search"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:53:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match stretched and medium size

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      60 |   })
      61 |
    > 62 |   it('have to match stretched and medium size', async () => {
         |   ^
      63 |     const screenshot = await testPageScreenshot({
      64 |       ...{ ...extend('input-medium'), style: { width: '300px' } },
      65 |       selector: '[data-visual-test="input-medium"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:62:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match stretched input with status

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      68 |   })
      69 |
    > 70 |   it('have to match stretched input with status', async () => {
         |   ^
      71 |     const screenshot = await testPageScreenshot({
      72 |       ...{ ...extend('input-stretch'), style: { width: '300px' } },
      73 |       selector: '[data-visual-test="input-stretch"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:70:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match error state

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      76 |   })
      77 |
    > 78 |   it('have to match error state', async () => {
         |   ^
      79 |     const screenshot = await testPageScreenshot({
      80 |       ...extend('input-error'),
      81 |       selector: '[data-visual-test="input-error"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:78:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match input with clear button

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      84 |   })
      85 |
    > 86 |   it('have to match input with clear button', async () => {
         |   ^
      87 |     const screenshot = await testPageScreenshot({
      88 |       selector: '[data-visual-test="input-clear"]',
      89 |     })

      at it (src/components/input/__tests__/Input.screenshot.test.js:86:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match input with clear button in hover state

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      91 |   })
      92 |
    > 93 |   it('have to match input with clear button in hover state', async () => {
         |   ^
      94 |     const screenshot = await testPageScreenshot({
      95 |       selector: '[data-visual-test="input-clear"]',
      96 |       simulateSelector:

      at it (src/components/input/__tests__/Input.screenshot.test.js:93:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match password input

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      101 |   })
      102 |
    > 103 |   it('have to match password input', async () => {
          |   ^
      104 |     const screenshot = await testPageScreenshot({
      105 |       ...extend('input-password'),
      106 |       selector: '[data-visual-test="input-password"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:103:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

  ● Input screenshot › have to match text align with icon

    thrown: "Exceeded timeout of 30000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      109 |   })
      110 |
    > 111 |   it('have to match text align with icon', async () => {
          |   ^
      112 |     const screenshot = await testPageScreenshot({
      113 |       ...extend('input-align'),
      114 |       selector: '[data-visual-test="input-align"]',

      at it (src/components/input/__tests__/Input.screenshot.test.js:111:3)
      at Object.describe (src/components/input/__tests__/Input.screenshot.test.js:11:1)
          at runMicrotasks (<anonymous>)

@langz langz force-pushed the feat/add-size-prop-to-date-picker branch from 40db891 to 02f0993 Compare June 10, 2022 11:43
@tujoworker tujoworker force-pushed the feat/add-size-prop-to-date-picker branch from 02f0993 to 31463e9 Compare June 10, 2022 12:33
Copy link
Contributor Author

@langz langz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the help here @tujoworker :)

@langz langz merged commit 749a118 into main Jun 10, 2022
@langz langz deleted the feat/add-size-prop-to-date-picker branch June 10, 2022 13:36
tujoworker pushed a commit that referenced this pull request Jun 23, 2022
# [9.27.0](v9.26.1...v9.27.0) (2022-06-23)

### Bug Fixes

* always hide content from DOM when Accordion is collapsed ([41cbd9b](41cbd9b))
* **Autocomplete:** enhance VoiceOver support ([#1472](#1472)) ([f469dd8](f469dd8))
* **Autocomplete:** ensure "no options" is read out in aria-live ([#1471](#1471)) ([254303f](254303f))
* **Autocomplete:** fix using tab + write issue ([#1473](#1473)) ([73f9655](73f9655))
* **Autocomplete:** make VoiceOver announce first if a certain item is selected ([#1479](#1479)) ([d32e980](d32e980))
* **Autocomplete:** touch device issue on first focus ([#1476](#1476)) ([5fcbf17](5fcbf17))
* **Avatar:** properly handle spacing props ([5841990](5841990))
* **Breadcrumb:** properly handle spacing props ([b713dad](b713dad))
* **DatePicker:** make keyboard usage in input not throw ([#1475](#1475)) ([d0756e2](d0756e2))
* **DatePicker:** rename testing attributes to data-testid ([2e1e285](2e1e285))
* **Dialog:** fix Modal backdrop / overlay false-positive click issue while e.g. selecting ([#1463](#1463)) ([91e69a8](91e69a8))
* **Dialog:** fix Safari issue not able to scroll ([3f6fb31](3f6fb31))
* **Dialog:** make Dialog not overflow iOS Safari viewport ([764e487](764e487))
* **Dropdown:** Missing properties in PropTypes ([#1384](#1384)) ([7a83b38](7a83b38))
* **HeightAnimation:** ensure quick state changes to react without animating from start to finish ([886b4cd](886b4cd))
* **HelpButton:** Adds test when used inside texts ([#1404](#1404)) ([f8a7795](f8a7795))
* **Icon:** fix stopwatch icon (16px) ([d57c617](d57c617))
* **Input:** add missing Input test for icon and clear button ([#1465](#1465)) ([be2b4a0](be2b4a0))
* **Input:** enhance vertical centering for Safari (mainly iOS) ([#1469](#1469)) ([dadb5f8](dadb5f8))
* **InputMasked:** fix integerLimit option when used with decimals ([14fe5d9](14fe5d9))
* **Modal:** fix data-testid usage in object defined props ([e023f13](e023f13))
* **Modal:** fix flaky overlay opacity color issue happening in Chrome browser ([86d875d](86d875d))
* **Radio:** fix radio button disabled issue when inside of group ([#1477](#1477)) ([e775792](e775792))
* **Table:** align HTML/CSS classes and types ([ff0be8d](ff0be8d))
* **Table:** fix sticky Table when no offset is given ([888564d](888564d))
* **Table:** simplify sticky table hook ([f1e7e3b](f1e7e3b))
* **Tag:** properly handle spacing props ([d7129d8](d7129d8))
* **Timeline:** properly handle spacing props ([e823c6e](e823c6e))

### Features

* **Autocomplete:** add support for data suffix_value ([#1467](#1467)) ([fe6fbb7](fe6fbb7))
* **DatePicker:** add size property to component ([#1438](#1438)) ([749a118](749a118))
* **DrawerList:** use CSS Grid for placing list content to enhance flexibility ([#1470](#1470)) ([93fac6b](93fac6b))
* **Icons:** add calendar and bookmark icon ([#1449](#1449)) ([61c9c11](61c9c11))
* **Input:** add inner_element property for internal usage (as of now) ([#1466](#1466)) ([d1b1b19](d1b1b19))
* **MediaQuery:** convert to TypeScript ([1a23e4f](1a23e4f))
* **Modal:** add "bypass_invalidation_selectors" property to omit element invalidation ([d9cb392](d9cb392))
* **NumberFormat:** add srLabel property to enhance more easily a11y for screen reader users ([#1480](#1480)) ([004d675](004d675))
* **Provider:** rewrite to TypeScript ([#1436](#1436)) ([66416d9](66416d9))
* **Table:** change colors of table ([1f3687f](1f3687f))
* **Table:** move to components and rewrite to TypeScript ([562940a](562940a))
* **Timeline:** date supports a list of dates ([#1464](#1464)) ([a5c1dcd](a5c1dcd))
* **Timeline:** rename name to title & date to subtitle with backwards compatibility ([#1468](#1468)) ([6607963](6607963))
@tujoworker
Copy link
Member

🎉 This PR is included in version 9.27.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
Development

Successfully merging this pull request may close these issues.

Add property size to DatePicker component
3 participants