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

WCAG Pagination: Added Button Support #9755

Merged
merged 5 commits into from May 15, 2024
Merged

WCAG Pagination: Added Button Support #9755

merged 5 commits into from May 15, 2024

Conversation

BrahimMahadi
Copy link
Contributor

What does this pull request (PR) do?

This pull request adds support for the button tag on pagination elements.

Additional information

General checklist

  • Updated pagination documentation to include the button support
  • Built and tested PR's code for proper functionality
  • Documentation is bilingual

@Garneauma
Copy link
Contributor

Pre-approved upon successful review.

@Garneauma Garneauma added this to the v4.0.80 milestone May 6, 2024
Garneauma
Garneauma requested changes May 6, 2024
View reviewed changes
Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

As discussed,

  • Optimize CSS by using SASS format and variables
  • Move working example to be the last on the page
  • Add scroll to top of pagination page when buttonsUI is activated

site/pages/docs/ref/paginate/paginate-en.hbs Outdated Show resolved Hide resolved
site/pages/docs/ref/paginate/paginate-fr.hbs Outdated Show resolved Hide resolved
src/plugins/paginate/_base.scss Show resolved Hide resolved
src/plugins/paginate/_base.scss Outdated Show resolved Hide resolved
src/plugins/paginate/_base.scss Outdated Show resolved Hide resolved
src/plugins/paginate/paginate-en.hbs Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
@duboisp
Copy link
Member

duboisp commented May 13, 2024

Pre-approved upon the requested change is completed and re-tested

@BrahimMahadi BrahimMahadi force-pushed the pagination-button branch 2 times, most recently from e54fc43 to 1f02a52 Compare May 14, 2024 14:06
Garneauma
Garneauma requested changes May 14, 2024
View reviewed changes
Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

Some styling optimization needed. However, great work converting your CSS to SASS!

src/plugins/paginate/_base.scss Outdated Show resolved Hide resolved
src/plugins/paginate/_base.scss Outdated Show resolved Hide resolved
src/plugins/paginate/_base.scss Outdated Show resolved Hide resolved
src/plugins/paginate/_base.scss Outdated Show resolved Hide resolved
Garneauma
Garneauma requested changes May 14, 2024
View reviewed changes
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
Garneauma
Garneauma previously approved these changes May 14, 2024
View reviewed changes
@Garneauma Garneauma dismissed their stale review May 14, 2024 19:52

Noticed something.

Garneauma
Garneauma requested changes May 14, 2024
View reviewed changes
Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

Just noticed that the buttonsUI parameter's default value is "false", not "true".

site/pages/docs/ref/paginate/paginate-en.hbs Outdated Show resolved Hide resolved
site/pages/docs/ref/paginate/paginate-fr.hbs Outdated Show resolved Hide resolved
duboisp
duboisp previously requested changes May 14, 2024
View reviewed changes
Copy link
Member

@duboisp duboisp 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 inline comments, a few adjustment is required.

Great work on this!!

cc @Garneauma

src/plugins/paginate/paginate.js Show resolved Hide resolved
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
src/plugins/paginate/paginate-fr.hbs Show resolved Hide resolved
src/plugins/paginate/paginate-en.hbs Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Show resolved Hide resolved
Garneauma
Garneauma requested changes May 15, 2024
View reviewed changes
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Show resolved Hide resolved
src/plugins/paginate/paginate.js Outdated Show resolved Hide resolved
src/plugins/paginate/paginate.js Show resolved Hide resolved
src/plugins/paginate/paginate-fr.hbs Show resolved Hide resolved
@Garneauma Garneauma dismissed duboisp’s stale review May 15, 2024 15:10

Changes requested have been done.

Garneauma
Garneauma approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@Garneauma Garneauma left a comment

Choose a reason for hiding this comment

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

Merci Brahim! Super travail.

@Garneauma Garneauma merged commit 37c1307 into wet-boew:master May 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Garneauma Garneauma Garneauma approved these changes

@duboisp duboisp duboisp left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v4.0.80
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@BrahimMahadi @Garneauma @duboisp