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

Example: New e-shop example #2434

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Dec 20, 2023

Note: This PR supersedes #1839 which was approved in light mode only (a11y and design)

Related issues

Closes #1560

Description

Things to know before reviewing it

  • The tags should wrap on 2 lines at max and have a horizontal scroll padding.
  • The phone images should respect a ratio of 3/4 and have a width of 4 columns of its parent.
  • Spacings between texts inside the main phone cards are 20px above, 5px between From and orange price and 0px between this price and the optionnal one under.
  • There are some issues with the font styles. They will mainly be tackled with font tokens once we implement them.
  • Many spacings will need to be reworked once the spacing tokens will be implemented.
  • The counter component will maybe be discussed inside a spec meeting to determine the behavior, the a11y and the different designs it could take.
  • Fine to write on .bg-danger
  • .border-supporting-* shouldn't exist since its example related. They have been transformed into .border-success and .border-info.
  • The design for the main phone cards as been a bit tweaked to make sure to have the same (or almost) markup in each card (the checkbox and button should appear 20px under the bottomest element above).
  • The stretched links should be used only if there's only one thing to handle. There should be non-cliquable elements around those.
  • I've added some skip links, feel free to add some if you can see some useful others.
  • I've added a back-to-top component.
  • Some colors have to be tweaked a bit since the design changed some color tokens.

Things to tackle outside of this PR

Saved information after the modification for the dark mode

1rst commit: 904c7a8
Commit with dark mode changes: db92da5

Motivation & Context

Types of change

  • New feature (non-breaking change which adds functionality)

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
  • (NA) I have added JavaScript unit tests to cover my changes
  • (NA) 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
  • (NA) My new component is added in Storybook
  • (NA) 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

@louismaximepiton louismaximepiton added examples color mode Temporary label to highlight color mode issues labels Dec 20, 2023
Copy link

netlify bot commented Dec 20, 2023

Deploy Preview for boosted ready!

Name Link
🔨 Latest commit 4e335a8
🔍 Latest deploy log https://app.netlify.com/sites/boosted/deploys/65b8a69e410fa100085fcf18
😎 Deploy Preview https://deploy-preview-2434--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.

@julien-deramond julien-deramond removed the color mode Temporary label to highlight color mode issues label Dec 21, 2023
@julien-deramond julien-deramond changed the title Examples: Implement e-shop dark mode Example: New e-shop example Dec 21, 2023
@julien-deramond julien-deramond force-pushed the main-lmp-e-shop-rebased branch 2 times, most recently from 5602e9b to ab8532f Compare December 27, 2023 10:58
Base automatically changed from main-jd-dark-mode to main January 2, 2024 14:21
First steps to have the card header

End of first draft

.

.

Some changes to respect a bit more the design

Some design fixes

.

.

fix(sonarCloud)

fix(pa11yCi)

better a11y

first batch

First release

Adding the necessary files

.

fix(review)

fix(review)

Implementing new version of the counter

Correct a Chrome bug

fix(a11y review)

fix(a11y review) + design changed

.

fix(a11y review)

fix(a11y review)

.
@julien-deramond
Copy link
Member

The mode selector should be in the header.
The dark mode version should be provided by the designers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Need Dev Review
Development

Successfully merging this pull request may close these issues.

E-Shop - Devices list
2 participants