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

[Research List] Fix initial state of filter and sorting #156

Merged
merged 3 commits into from
Jun 20, 2024
Merged

[Research List] Fix initial state of filter and sorting #156

merged 3 commits into from
Jun 20, 2024

Conversation

hsinlun99
Copy link
Contributor

@hsinlun99 hsinlun99 commented Jun 20, 2024 •
edited
Loading

Description

  • initialize sorting and filter state after closing cluster drawer
  • close drawer isListShown state after closing cluster drawer

Changes

  • filter
    • 因為是 redux 實作的，就把相關的 dispatch 丟到 userFab 的回主頁面那邊了
  • sorting method
    • 他是只存在於 cluster drawer 中的 state，跟著 cluster drawer 原有的另一個判斷 cluster drawer 是否關閉 (searchParams 判斷) 的 useEffect 去重設
  • isListShown
    • 在 drawer 多聽了一個 URL 判斷是否是 cluster 並 initialize isListShown
    • Improvement needed. add/edit/poi Drawer 和 cluster Drawer 應該要分開而不是用同一個 template 了

Notes

Checklist

  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have checked my code and corrected any misspellings
  • Any changes to strings have been published to our translation tool
  • I conducted basic QA to assure all features are working
  • I requested code review from other team members

Resolved Issues

@hsinlun99 hsinlun99 requested a review from linyc0817 June 20, 2024 13:18
@hsinlun99 hsinlun99 marked this pull request as ready for review June 20, 2024 13:18
linyc0817
linyc0817 approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@linyc0817 linyc0817 left a comment

Choose a reason for hiding this comment

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

LGTM

@linyc0817 linyc0817 merged commit 830d166 into research-list Jun 20, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linyc0817 linyc0817 linyc0817 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@hsinlun99 @linyc0817