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.

Sign up for GitHub

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

Jump to bottom

Pagination: responsive part round 2 #1931

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

Pagination: responsive part round 2 #1931

wants to merge 36 commits into from

Conversation

louismaximepiton
Copy link
Member

@louismaximepiton louismaximepiton commented Mar 29, 2023 •
edited
Loading

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

Related issues

Superseed #1370.

Description

Add something to handle the mobile responsive part

Motivation & Context

Follow the Orange brand.

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) -> Is well for: Chrome 88+, FF 84+, Edge 88+, Safari 12+, phones, apprently due to :not but already used almost everywhere and should be supported by all browser, I might have done something wrong in here. (https://developer.mozilla.org/en-US/docs/Web/CSS/:not#browser_compatibility)
  • Code review
  • Design review
  • A11y review

After the merge

louismaximepiton and others added 29 commits July 6, 2022 17:43
@louismaximepiton
First step
b1f8549
@louismaximepiton
Second and third step for the mixin first attempt
f7536d9
@louismaximepiton
Patch for too few elements
f9f30ed
@louismaximepiton
Fix for something I didn't noticed + lighten the bundle
fdf56bc
@louismaximepiton
Decreasing package size while upgrading functionnality
2ebd723
@louismaximepiton
.
6c965dc
@julien-deramond
Merge branch 'main' into main-lmp-responsive-pagination
30da0be
@julien-deramond
Merge branch 'main' into main-lmp-responsive-pagination
03a120b
@julien-deramond
Merge branch 'main' into main-lmp-responsive-pagination
738be90
@louismaximepiton
Working pagination in all cases with doc
4a5907f
@louismaximepiton
Merge remote-tracking branch 'origin/main' into main-lmp-responsive-p…
f7cdf56 
…agination
@louismaximepiton
.
31590c8
@julien-deramond
Merge branch 'main' into main-lmp-responsive-pagination
e603851
@louismaximepiton
Having an opt-in to activate/deactivate the responsive behavior
bf8d0f3
@louismaximepiton
Better comments
ae919ad
@louismaximepiton
.
8e7bd72
@louismaximepiton
All good
853f43d
@julien-deramond
Merge branch 'main' into main-lmp-responsive-pagination
0d729e7
@louismaximepiton
fix(review)
8daa5ee
@julien-deramond
Add some Boosted mod, comments and modified slightly the docs descrip…
42772fa 
…tions
@julien-deramond
Minor wording modification found while checking for typos
4a357bb
@louismaximepiton
Merge branch 'main' into main-lmp-responsive-pagination
9d2d338
@louismaximepiton
Try ?
a193067
@louismaximepiton
Merge branch 'main' into main-lmp-responsive-pagination
3a47ae6
@louismaximepiton
fix(review)
f6b91e6
@louismaximepiton
Merge branch 'main' into main-lmp-responsive-pagination
7215620
@louismaximepiton
Merge branch 'main' into main-lmp-responsive-pagination
314814d
@louismaximepiton
forgot about it
f5c8866
@louismaximepiton
First draft almost convincing to me
801390b
@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 March 30, 2023 08:06
@louismaximepiton louismaximepiton mentioned this pull request Mar 30, 2023
Closed
12 tasks
MewenLeHo
MewenLeHo reviewed Apr 4, 2023
View reviewed changes
Copy link
Contributor

@MewenLeHo MewenLeHo left a comment

Choose a reason for hiding this comment

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

Possible a11y issue, need to check with our experts.

MewenLeHo
MewenLeHo approved these changes Apr 18, 2023
View reviewed changes
Copy link
Contributor

@MewenLeHo MewenLeHo left a comment

Choose a reason for hiding this comment

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

🚀

@Aniort Aniort self-requested a review April 21, 2023 14:07
Aniort
Aniort approved these changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@Aniort Aniort left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As @MewenLeHo says, there content conveying info display via CSS (< and >) it's a n issue
add a real icon with an aria-hidden=true

@Aniort Aniort self-requested a review April 21, 2023 14:21
Aniort
Aniort suggested changes Apr 21, 2023
View reviewed changes
Copy link
Contributor

@Aniort Aniort left a comment

Choose a reason for hiding this comment

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

see my comment before

@louismaximepiton louismaximepiton mentioned this pull request Apr 24, 2023
Open
2 tasks
@louismaximepiton
Copy link
Member Author

Will be taken into account with #2008.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 3, 2023

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

@Franco-Riccitelli
Copy link
Member

The third example for pagination Responsive behaviour looks and behaves as specified. No issues.

@MewenLeHo MewenLeHo mentioned this pull request Aug 8, 2023
Open
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Aniort Aniort Aniort requested changes

@MewenLeHo MewenLeHo MewenLeHo approved these changes

Assignees
No one assigned
Labels
brand ⚛️ component 🧩 enhancement 👌 OK Accessibility 👌 OK Design v5
Projects
🟣 [Orange-Boosted-Bootstrap] PRs Board
Status: Need Lead Dev Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@louismaximepiton @Franco-Riccitelli @Aniort @MewenLeHo @julien-deramond