Skip to content
This repository has been archived by the owner on May 2, 2023. It is now read-only.

feat(form-elements): Refactoring form elements components - FRONT-1029 #406

Merged
merged 12 commits into from
Apr 8, 2020

Conversation

planctus
Copy link
Contributor

@planctus planctus commented Apr 8, 2020

PR description

Please drop a few lines about the PR: what it does, how to test it, etc.

QA Checklist

In order to ensure a safe and quick review, please check that your PR follow those guidelines:

  • I have put the vanilla component as devDependencies
  • I have put the specs package as devDependencies
  • I have added the components directly used in the twig file (with include or embed) as dependencies
  • My component is listed in @ecl-twig/ec-components's dependencies
  • My variables naming follow the guidelines (snake case for twig)
  • I have provided tests
  • I have provided documentation (for the "notes" tab)
  • If my local yarn.lock contains changes, I have committed it
  • I have given my PR the proper label (pr: review needed to indicate that I'm done and now waiting for a review ,pr: wip to indicate that I'm actively working on it ...)

@planctus planctus added pr: review needed Use this label to show that your PR needs to be review tag: enhancement labels Apr 8, 2020
@papegaill papegaill added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Apr 8, 2020
Copy link
Contributor

@papegaill papegaill left a comment

Choose a reason for hiding this comment

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

Here are 2 issues I spotted so far, for the rest It looks good to me..

https://5e8dd4b735aaf44a4e9a6723--ecl-twig.netlify.com/ec/?path=/story/components-forms-radio--binary

Cannot read property 'replace' of undefined
TypeError: Cannot read property 'replace' of undefined

@@ -584,7 +584,7 @@ exports[`EC - checkbox Invalid renders correctly 1`] = `
value="es"
/>
<label
class="ecl-form-label ecl-checkbox__label ecl-form-label--invalid"
class="ecl-checkbox__label checkbox__label--invalid"
Copy link
Contributor

Choose a reason for hiding this comment

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

checkbox__label--invalid seems strange? can't find correspondance in ecl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed

hidden=""
id="expandable-example-content"
>
The EU is building an energy union that make sures Europe’s energy supply is safe, viable and accessible to all. In doing so, it can boost the economy and attract investments that can create new jobs opportunities.
<p
class="ecl-u-type-paragraph-m"
Copy link
Contributor

Choose a reason for hiding this comment

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

;)

@papegaill papegaill merged commit 2636eed into develop Apr 8, 2020
@papegaill papegaill deleted the FRONT-1029 branch April 8, 2020 15:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants