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

refactor(demo): hero-banner optional/mandatory knobs - FRONT-912 #373

Merged
merged 15 commits into from Mar 20, 2020

Conversation

Joosthe
Copy link
Contributor

@Joosthe Joosthe commented Mar 17, 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 ...)

@Joosthe Joosthe added pr: review needed Use this label to show that your PR needs to be review tag: enhancement labels Mar 17, 2020
@planctus planctus added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Mar 17, 2020
@Joosthe Joosthe added pr: review needed Use this label to show that your PR needs to be review and removed pr: modification needed labels Mar 18, 2020
@planctus planctus added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Mar 19, 2020
@Joosthe Joosthe added pr: review needed Use this label to show that your PR needs to be review and removed pr: modification needed labels Mar 19, 2020
@planctus planctus added pr: under review and removed pr: review needed Use this label to show that your PR needs to be review labels Mar 19, 2020
@Joosthe Joosthe changed the title refactor(demo): herobanner prepared - FRONT-912 refactor(demo): hero-banner optional/mandatory knobs - FRONT-912 Mar 19, 2020
@planctus planctus added pr: review needed Use this label to show that your PR needs to be review and removed pr: under review labels Mar 19, 2020
@Joosthe
Copy link
Contributor Author

Joosthe commented Mar 20, 2020

@planctus Looks really much improved and thanks for all the work, the only thing i could find on the component is that the icon is missing now in the preview of the component

@Joosthe Joosthe added pr: modification needed and removed pr: review needed Use this label to show that your PR needs to be review labels Mar 20, 2020
@planctus
Copy link
Contributor

No, @Joosthe , the icon is there:
https://5e73a38ee6b9de0ee0b17d46--ecl-twig.netlify.com/ec/?path=/story/components-banners-hero-banner--image
I think you just need to update the plugin for the story-utils, sorry not to point this out in advance, but as usual, when we have changes in one of our plugins we need to pay attention to really update it on our local environments.
So, you can remove the node-modules folder and run yarn again or you can copy the index.js file from the working tree (utils/story-utils/src/index.js) to the package in the node_modules folder (node-modules/@ecl-twig/story-utils/src/index.js).
I hope this is the last time i have to write this, but it's true that might maybe find a solution for this beacuse i see that everytime is an issue.

@planctus planctus added pr: review needed Use this label to show that your PR needs to be review and removed pr: modification needed labels Mar 20, 2020
@Joosthe Joosthe merged commit 77e07ae into develop Mar 20, 2020
@Joosthe Joosthe deleted the FRONT-912 branch March 20, 2020 08:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr: review needed Use this label to show that your PR needs to be review tag: enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants