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

Use replace for pagination navigation #640

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

ykeremy
Copy link
Contributor

@ykeremy ykeremy commented Jul 24, 2024

🚀 This description was created by Ellipsis for commit 73ef82c

Summary:

Updated pagination navigation to use replace option in setSearchParams to prevent cluttering browser history.

Key points:

  • Updated pagination navigation to use replace option in setSearchParams.
  • Affected files: skyvern-frontend/cloud/routes/billing/BillingHistory.tsx, skyvern-frontend/src/routes/tasks/list/TaskHistory.tsx, skyvern-frontend/src/routes/workflows/WorkflowPage.tsx, skyvern-frontend/src/routes/workflows/Workflows.tsx.
  • Ensures browser history is not cluttered with pagination changes.

Generated with ❤️ by ellipsis.dev

…src/'

<!-- ELLIPSIS_HIDDEN -->

| 🚀 | This description was created by [Ellipsis](https://www.ellipsis.dev) for commit 3b9fd0f54963fdefc1a54ceebd2696bd9223f79a  |
|--------|--------|

### Summary:
Updated pagination navigation to use `replace` option in `setSearchParams` to prevent cluttering browser history.

**Key points**:
- Updated pagination navigation to use `replace` option in `setSearchParams`.
- Affected files: `skyvern-frontend/cloud/routes/billing/BillingHistory.tsx`, `skyvern-frontend/src/routes/tasks/list/TaskHistory.tsx`, `skyvern-frontend/src/routes/workflows/WorkflowPage.tsx`, `skyvern-frontend/src/routes/workflows/Workflows.tsx`.
- Ensures browser history is not cluttered with pagination changes.

----
Generated with ❤️ by [ellipsis.dev](https://www.ellipsis.dev)

<!-- ELLIPSIS_HIDDEN -->
@ykeremy ykeremy added the sync label Jul 24, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 73ef82c in 28.50763 seconds

More details
  • Looked at 101 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/Workflows.tsx:129
  • Draft comment:
    The implementation of setSearchParams with the replace option is correctly applied here to avoid adding new entries to the browser's history stack when navigating through pagination. This is a good practice for enhancing user experience in applications with heavy navigation elements like pagination.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR aims to update the pagination navigation to use the replace option in setSearchParams to prevent cluttering the browser history. This is a good practice for pagination where the user might click through many pages and does not need each page to be stored in the history stack. The changes are consistent across multiple files and correctly implement the replace option. The PR description matches the changes made in the code.

Workflow ID: wflow_FGW0VKdyHxNcGJtf


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Incremental review on 73ef82c in 36.604859 seconds

More details
  • Looked at 100 lines of code in 3 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/Workflows.tsx:129
  • Draft comment:
    The implementation of setSearchParams with the replace: true option across various files correctly aligns with the PR's goal to prevent cluttering the browser history during pagination. This is a good practice for enhancing user experience.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR description indicates that the changes are intended to use the replace option in setSearchParams to prevent cluttering the browser history when navigating pagination. This is a good practice for enhancing user experience by not filling the history stack with every page change in pagination.

The changes in the PR correctly implement the replace: true option in all instances where setSearchParams is used for pagination across multiple files. This ensures that the browser history will not be cluttered with each pagination step, which aligns with the stated goal of the PR.

Overall, the implementation across different files is consistent and adheres to the best practices of not affecting the browser history unnecessarily during pagination navigation.

Workflow ID: wflow_nPiYF7oF7cRip9LL


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@msalihaltun msalihaltun merged commit 1dc37b3 into main Jul 24, 2024
2 checks passed
@msalihaltun msalihaltun deleted the salih/use-replace-in-query-param-navigation branch July 24, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants