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

SSINTER-253: Versioned snapshots > CMS 5 upgrade > Pagination #105

Conversation

poyjavier
Copy link

SSINTER-253: Versioned snapshots > CMS 5 upgrade > Pagination

Bug fixes:

  • Fixed version list not displayed when it is more than 30 items
  • Fixed pagination not working
  • Removed Griddle. Only used before to match styling of ThumbnailView.js (Assuming this component's pagination not working too due to griddle-react@1.13.1 new architecture.)
  • Added custom pagination with styling that matches the old one.
  • Confirmed SS CMS 5 compatibility.

Issues

Checklist:

  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
  • CI is green

Copy link
Collaborator

@mfendeksilverstripe mfendeksilverstripe left a comment

Choose a reason for hiding this comment

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

Looks like you're missing the dist files. Please make sure to build the dist files and commit the changes.

Copy link
Collaborator

@mfendeksilverstripe mfendeksilverstripe left a comment

Choose a reason for hiding this comment

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

Overall it's looking good 👍 Left some comments to follow up on though.

@@ -151,3 +151,44 @@
}
}

.griddle-footer {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm a bit concerned about the scope of these styles. Maybe we can limit this by wrapping all of the new styles within .history-viewer?

key={pageNumber}
className={`page-item ${pageNumber === page ? 'active' : ''}`}
>
<button
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed that clicking pagination button within the inline editor causes to trigger block save action. This is not the case for previous and next button for some reason. The pagination should not be triggering any other actions. Can you please check what's causing this?

Screenshot 2025-01-14 at 8 41 13 AM

Copy link
Author

Choose a reason for hiding this comment

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

@mfendeksilverstripe Can you please provide the steps to replicate this? Not able to replicate this behaviour. Cheers

Copy link
Collaborator

Choose a reason for hiding this comment

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

As per our conversation, we will separate this issue out.

@poyjavier
Copy link
Author

Better approach fix which aligns styling of other griddle tables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants