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

Storybook: enhance story making #2130

Merged
merged 63 commits into from
Jul 21, 2023
Merged

Conversation

louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Jul 6, 2023

Note: Please transform - [ ] into - (NA) in the description when things are not applicable

Related issues

Fixes some points in #1626.

Description

⚠️ Merge after #2103.

Take all components in the folder directly, so we don't need to update this file every time we need it.
Try to take some Js automatically from snippets.js.

Motivation & Context

Automate our processes.

Types of change

  • Refactoring (non-breaking change)

Live previews

Checklist

Contribution

Accessibility

  • My change follows accessibility good practices; I have at least run axe

Design

  • My change respects the design guidelines defined in Orange Design System
  • My change is compatible with responsive display

Development

  • My change follows the developer guide
  • I have added JavaScript unit tests to cover my changes
  • I have added SCSS unit tests to cover my changes

Documentation

  • My change introduces changes to the documentation and/or I have updated the documentation accordingly

Checklist (for Core Team only)

  • My change introduces changes to the migration guide
  • My new component is added in Storybook
  • My new component is compatible with RTL
  • Manually run BrowserStack tests
  • Manually test browser compatibility with BrowserStack (Chrome >= 60, Firefox >= 60 (+ ESR), Edge, Safari >= 12, iOS Safari, Chrome & Firefox on Android)
  • Code review
  • Design review
  • A11y review

After the merge

@netlify
Copy link

netlify bot commented Jul 6, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 84bc74a
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/64ba792391e03000080a2ef9
😎 Deploy Preview https://deploy-preview-2130--boosted.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@louismaximepiton louismaximepiton mentioned this pull request Jul 10, 2023
12 tasks
@louismaximepiton
Copy link
Member Author

louismaximepiton commented Jul 10, 2023

Tracking of the components:

Thing to know before reviewing, it's the code from main with the look of 5.2. It'll change once the 5.3 is released.

⚠️ Issue when clicking on a submit button but might be fine. Should I

Accordion

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Alerts

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ⚠️The Alert live triggering is triggered the number of example -> Need to refactor snippets.js but might be done outside the PR

BackToTop

  • Rendering ✔️
  • Manipulation ⚠️ We disable the default behavior so the manipulation doesn't work.
  • Docs ✔️

Badge

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Breadcrumb

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

ButtonGroup

  • Rendering ✔️ [Out of PR] 10: open a dropdown and hover a button beside renders weird.
  • Manipulation ✔️
  • Docs ⚠️ Some issues with dropdowns due to iframes, I don't think we can handle this on our side.

Buttons

  • Rendering ✔️
    • 0: Base class comes with 5.3, so it'll be updated once the 5.3 is out. ⚠️This might shift the URL used.
    • 5: Social TikTok comes with 5.3, so it'll be updated once the 5.3 is out.
    • 8: Social TikTok comes with 5.3, so it'll be updated once the 5.3 is out.
    • 10: Secondary outline comes with 5.3, so it'll be updated once the 5.3 is out. ⚠️This might shift the URL used.
  • Manipulation ✔️
  • Docs ✔️

Card

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Carousel

  • Rendering ✔️
    • 2: Play-pause changed with 5.3, so it'll be updated once the 5.3 is out.
    • 5: ⚠️This might shift the URL used.
    • 6: ⚠️This might shift the URL used.
  • Manipulation ⚠️ The instanciation doesn't seem to work as expected (indicators rolling and sliding aren't good). Seems to be a version issue here.
    • 2: Play button isn't accessible atm due to design modifications with 5.3, so it'll be updated once the 5.3 is out.
  • Docs ✔️

ChecksRadios

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

CloseButton

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Collapse

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Dropdowns

  • Rendering ✔️
    • 6: Dropdown in navbar changed with 5.3, so it'll be updated once the 5.3 is out.
    • 8, 9, 19: Dropup have some issues rendering inside iframe, I don't think we can handle this on our side.
    • 11: Dropstart have some issues rendering inside iframe, I don't think we can handle this on our side.
    • 22: .text-muted changed with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ⚠️ Some issues with dropdowns due to iframes, I don't think we can handle this on our side.

Footer

  • Rendering ✔️
    • 5, 6: Active state comes with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ✔️

FormControl

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

InputGroup

  • Rendering ✔️
    • 0: .form-text comes with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ⚠️ Some issues with dropdowns due to iframes, I don't think we can handle this on our side.

Layout

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

ListGroup

  • Rendering ✔️
    • 10: Variants seems to have changed with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ✔️

Modal

  • Rendering ✔️
  • Manipulation 🔴 Due to the way Modals are implemented.
    • 2-8, 11-12: Modal not inside the example
  • Docs ⚠️ The backdrop seems to go above the iframes.

Navbar

  • Rendering ✔️
    • 19: .text-muted changed with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ⚠️ The offcanvases (20-21) seems to be a bit broken on the rendering inside the docs.

NavsTabs

  • Rendering ✔️
    • 7: ⚠️The doc has been reordered. This might shift the URL used.
    • 8-9: .nav-underline has been introduced over .navs-tabs-light with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ⚠️ Some issues with dropdowns due to iframes, I don't think we can handle this on our side.

Offcanvas

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ⚠️ The offcanvases (1-4, 6-9) don't show. Due to the implementation that avoids several offcavas instances to be open at the same time. IMO, this is an edge case that don't need to be resolved.

OrangeNavbar

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Overview

  • Rendering ✔️
    • 0: .form-text comes with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️ Tooltip and popover check
  • Docs ✔️

Pagination

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Placeholders

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Popovers

⚠️ Changing the page don't dismiss the popovers.

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Progress

  • Rendering ✔️
    • 2, 3:⚠️The doc has been reordered. This might shift the URL used.
    • 4, 7, 8:⚠️New example in 5.3. This might shift the URL used.
    • 9: .progress-stacked introduced with 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ✔️

QuantitySelector

  • Rendering ✔️
    • 0, 1: The buttons - aren't well initialized. I don't think we can handle this.
  • Manipulation ✔️
  • Docs ✔️

Range

  • Rendering ✔️
    • 4: The output isn't well displayed. The js might need an updated version to work wth SB ?
  • Manipulation ✔️
  • Docs ✔️

Scrollspy

  • Rendering 🔴 Scrollspies aren't initialized
  • Manipulation 🔴 Can't test it
  • Docs

Select

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Spinners

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

SteppedProcess

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Sticker

  • Rendering ⚠️
    • 0-3: .sticker-fs-* has been introduced within 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ✔️

Tables

  • Rendering ✔️
    • 14: .visually-hidden has been changed within 5.3, so it'll be updated once the 5.3 is out.
  • Manipulation ✔️
  • Docs ✔️

Tags

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

TitleBars

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Toasts

  • Rendering ✔️
  • Manipulation ✔️
    • 1: The button doesn't for the same reason as modals.
  • Docs ✔️

Tooltips

  • Rendering ✔️
  • Manipulation ✔️
  • Docs ✔️

Validation

  • Rendering ✔️
  • Manipulation 🔴 Clicking on a submit button send to another pageI don't think we can handle this that easily.
  • Docs ✔️

@louismaximepiton louismaximepiton force-pushed the main-lmp-add-js-storybook branch from 67905a2 to d41146f Compare July 11, 2023 07:00
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@louismaximepiton louismaximepiton marked this pull request as ready for review July 13, 2023 07:34
@julien-deramond julien-deramond mentioned this pull request Jul 17, 2023
5 tasks
Copy link
Contributor

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

A few quick comments before continuing with the complete review.

.github/PULL_REQUEST_TEMPLATE.md Show resolved Hide resolved
.storybook/preview-head.html Outdated Show resolved Hide resolved
stories/create-stories-from-doc.js Show resolved Hide resolved
site/assets/js/snippets.js Outdated Show resolved Hide resolved
@julien-deramond

This comment was marked as resolved.

Copy link
Contributor

@julien-deramond julien-deramond left a comment

Choose a reason for hiding this comment

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

So cool! Great improvement! 🚀

@julien-deramond julien-deramond merged commit 79d9b99 into main Jul 21, 2023
@julien-deramond julien-deramond deleted the main-lmp-add-js-storybook branch July 21, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

2 participants