Skip to content

fix: switch saveMode for SaveFilterSegmentButton#20502

Merged
CarinaWolli merged 2 commits intomainfrom
eunjae/cal-5398-fix-bugs-with-filter-segment
Apr 2, 2025
Merged

fix: switch saveMode for SaveFilterSegmentButton#20502
CarinaWolli merged 2 commits intomainfrom
eunjae/cal-5398-fix-bugs-with-filter-segment

Conversation

@eunjae-lee
Copy link
Contributor

What does this PR do?

This PR fixes the SaveFilterSegmentButton.

The saveMode should change based on whether a filter segment is currently selected. This PR updates the saveMode value accordingly.

  • Fixes CAL-5398

Visual Demo (For contributors especially)

Because of this bug, we had this issue:

Screenshot 2025-04-02 at 10 31 01

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • N/A - I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Go to org member list
  • Create a filter segment
  • Make some changes to enable "Save" button
  • Click the "Save" button
  • The name <input> won't show up (it used to show up occasionally)

@linear
Copy link

linear bot commented Apr 2, 2025

@keithwillcode keithwillcode added consumer core area: core, team members only labels Apr 2, 2025
const setPageSizeAndGoToFirstPage = useCallback(
(newPageSize: number | null) => {
setPageSize(newPageSize === DEFAULT_PAGE_SIZE ? null : newPageSize);
setPageSize(newPageSize === defaultPageSize ? null : newPageSize);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should use the actual defaultPageSize given from the caller.

columnVisibility,
columnSizing,
perPage: 10,
perPage: pageSize,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We didn't use the actual value here

@eunjae-lee eunjae-lee marked this pull request as ready for review April 2, 2025 08:44
@graphite-app graphite-app bot requested a review from a team April 2, 2025 08:44
@graphite-app
Copy link

graphite-app bot commented Apr 2, 2025

Graphite Automations

"Add consumer team as reviewer" took an action on this PR • (04/02/25)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add ready-for-e2e label" took an action on this PR • (04/02/25)

1 label was added to this PR based on Keith Williams's automation.

@vercel
Copy link

vercel bot commented Apr 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Apr 2, 2025 8:53am
calcom-web-canary ⬜️ Ignored (Inspect) Visit Preview Apr 2, 2025 8:53am

@CarinaWolli CarinaWolli enabled auto-merge (squash) April 2, 2025 09:28
@CarinaWolli CarinaWolli merged commit 9ac037c into main Apr 2, 2025
60 of 61 checks passed
@CarinaWolli CarinaWolli deleted the eunjae/cal-5398-fix-bugs-with-filter-segment branch April 2, 2025 09:58
@github-actions
Copy link
Contributor

github-actions bot commented Apr 2, 2025

E2E results are ready!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working consumer core area: core, team members only ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants