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

Quantity Selector #860

Merged
merged 35 commits into from
Feb 24, 2022
Merged

Quantity Selector #860

merged 35 commits into from
Feb 24, 2022

Conversation

MewenLeHo
Copy link
Contributor

@MewenLeHo MewenLeHo commented Oct 20, 2021 •
edited by julien-deramond

Closes #113

Preview: https://deploy-preview-860--boosted.netlify.app/docs/5.1/forms/quantity-selector/

DoD

Development

  • Should match specs (eg. either the Web UI Kit or any pattern from the WAI — or both…)
  • Docs added:
    • including the "Sass" part using scss-docs shortcode
    • in Cheatsheet / Cheatsheet RTL examples
    • in /about/overview/#custom-components if it is a new Orange custom component
    • in /gettings-started/introduction/#components if it is a new Orange custom component that requires JavaScript (and Popper)
    • in /customize/overview#csps-and-embedded-svgs if it is a new Orange custom component that includes embedded SVGs in our CSS
    • in /forms/validation/?#supported-elements if it is a new Orange custom component that is a form control
  • Check (and fix) RTL version
  • Run linters
  • Run compilers
  • Tests added for JS-side
  • Run tests
  • Cross-browser test:
    • Firefox ESR
    • Latest Edge, Chrome, Firefox, Safari
    • iOS Safari
    • Chrome & Firefox on Android
  • Clean up the branch using rebase -i
  • Commited with feat(…): … message
  • Mention it in Migration Guide (if back-from-v4): renamed variables, changes in markup requirement, etc.

Reviews

  • Code review
  • A11y review
  • Design review

@MewenLeHo MewenLeHo self-assigned this Oct 20, 2021
@MewenLeHo MewenLeHo marked this pull request as draft October 20, 2021 15:21
@julien-deramond julien-deramond removed their request for review October 20, 2021 16:01
@MewenLeHo MewenLeHo force-pushed the main-mewenleho-quantityselector branch from 53a434a to 3d5c7ca Compare November 15, 2021 10:08
@louismaximepiton louismaximepiton mentioned this pull request Nov 16, 2021
Closed
@MewenLeHo MewenLeHo force-pushed the main-mewenleho-quantityselector branch from 179014d to d431c0b Compare November 16, 2021 15:52
@MewenLeHo MewenLeHo marked this pull request as ready for review November 18, 2021 14:59
@MewenLeHo MewenLeHo force-pushed the main-mewenleho-quantityselector branch 4 times, most recently from fe2f94b to 65510a5 Compare November 29, 2021 09:22
@MewenLeHo MewenLeHo force-pushed the main-mewenleho-quantityselector branch from 5e35aeb to 28ab547 Compare December 7, 2021 08:36
@MewenLeHo MewenLeHo force-pushed the main-mewenleho-quantityselector branch from 5fbcf1f to c25858d Compare December 15, 2021 08:49
julien-deramond
julien-deramond requested changes Dec 16, 2021
View reviewed changes
Copy link
Member

@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.

Here is my first feedback for this component. I haven't yet looked at it in details.

You will find the DoD in the description: just check if you don't need to reference the component in other parts of the documentation.

Some unit tests would be appreciated.

js/src/quantity-selector.js Show resolved Hide resolved
js/src/quantity-selector.js Show resolved Hide resolved
js/src/quantity-selector.js Outdated Show resolved Hide resolved
js/src/quantity-selector.js Outdated Show resolved Hide resolved
scss/_variables.scss Outdated Show resolved Hide resolved
scss/_variables.scss Outdated Show resolved Hide resolved
scss/forms/_quantity-selector.scss Show resolved Hide resolved
site/content/docs/5.1/forms/quantity-selector.md Outdated Show resolved Hide resolved
site/static/docs/5.1/assets/img/boosted-sprite.svg Outdated Show resolved Hide resolved
@MewenLeHo MewenLeHo force-pushed the main-mewenleho-quantityselector branch 2 times, most recently from b61f8ff to 1775750 Compare December 21, 2021 14:51
@MewenLeHo MewenLeHo force-pushed the main-mewenleho-quantityselector branch from 1775750 to 1129e73 Compare December 22, 2021 10:47
@julien-deramond

This comment has been minimized.

Sign in to view
julien-deramond
julien-deramond approved these changes Dec 27, 2021
View reviewed changes
Copy link
Member

@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.

Made some modifications in order to:

  • Improve the doc and the formatting
  • Fix some issues in .scss files and rendering
  • Add some unit tests
    Those modifications are the last 4 commits of this PR. Please tell me @MewenLeHo if it is OK for you. Perhaps we need to ask for a new design review before merging?

MewenLeHo and others added 21 commits February 24, 2022 12:13
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix typo and job
c6ab966
@MewenLeHo @julien-deramond
feat #113 (quantity selector): add more unit tests
8671426
@julien-deramond
Fix bundlewatch conflict
bc72706
@julien-deramond
Use style inherited from Bootstrap
4616a18
@julien-deramond
Fix bundlewatch
1dca654
@julien-deramond
Finish formatting JS file + clean up Sass variables modifications
bbe452a
@MewenLeHo @julien-deramond
feat #113 (quantity selector): add jQueryInterface
a89537a
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix bundlewatch
00c8dff
@MewenLeHo @julien-deramond
feat #113 (quantity selector): add dispatchEvent on StepUp and StepDown
bfc8a87
@MewenLeHo @julien-deramond
feat #113 (quantity selector): add jQuery part in doc
8c1c4db
@MewenLeHo @julien-deramond
feat #113 (quantity selector): refactoring js
f11981e
@MewenLeHo @julien-deramond
feat #113 (quantity selector): back on refactoring js
ac462a1
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix bundlewatch
5a3fad2
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix load event in unit tests
cac060b
@MewenLeHo @julien-deramond
feat #113 (quantity selector): add jQueryInterface unit tests
77b8c57
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix case
de4c7ec
@MewenLeHo @julien-deramond
feat #113 (quantity selector): update javascript usage documentation
9630d96
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix bundlewatch
cf147c4
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix documentation for js change event
126fa7c
@MewenLeHo @julien-deramond
feat #113 (quantity selector): fix bundlewatch
a648d24
@MewenLeHo @julien-deramond
feat #113 (quantity selector): minor fixes after review
34ad178
julien-deramond
julien-deramond reviewed Feb 24, 2022
View reviewed changes
scss/forms/_quantity-selector.scss Outdated Show resolved Hide resolved
scss/forms/_quantity-selector.scss Outdated Show resolved Hide resolved
julien-deramond
julien-deramond approved these changes Feb 24, 2022
View reviewed changes
Copy link
Member

@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.

Sounds good to me. Good job 🚀

@julien-deramond julien-deramond merged commit 1bfc839 into main Feb 24, 2022
@julien-deramond julien-deramond deleted the main-mewenleho-quantityselector branch February 24, 2022 15:05
@julien-deramond julien-deramond mentioned this pull request Feb 24, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@louismaximepiton louismaximepiton louismaximepiton left review comments

@julien-deramond julien-deramond julien-deramond approved these changes

Assignees

@MewenLeHo MewenLeHo

Labels
feature v5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Forms > Quantity selector
4 participants
@MewenLeHo @julien-deramond @Franco-Riccitelli @louismaximepiton