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

refactor (search filter) : syncing main tytype chips and bottom dialog tytype chips #811

Merged
merged 4 commits into from Dec 15, 2023

Conversation

ghost
Copy link

@ghost ghost commented Dec 12, 2023

changes applicable to search page.

  • and run search when bottom dialog for provider filter closes <- should this be on dialog close or selecting tytype chips in dialog?

image

@ghost ghost changed the title refactor (search filter) : main selected chips are synced when selecting from bottom dialog chips refactor (search filter) : main selected chips are synced when selecting from bottom provider filter dialog chips Dec 12, 2023
@ghost ghost changed the title refactor (search filter) : main selected chips are synced when selecting from bottom provider filter dialog chips refactor (search filter) : syncing main tytype chips and bottom dialog tytype chips Dec 12, 2023
Copy link
Collaborator

@fire-light42 fire-light42 left a comment

Choose a reason for hiding this comment

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

This code contains a bug causing the chips to stop working

selectedSearchTypes,
validAPIs.flatMap { api -> api.supportedTypes }.distinct()
) {
// This already handled in another bindChips. Do nothing here!
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, this will break the logic as the callback (button?.setOnCheckedChangeListener) will be set to Unit and the buttons will stop working. If you want to keep the prev callback make the callback nullable

Copy link
Author

@ghost ghost Dec 14, 2023

Choose a reason for hiding this comment

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

no need for another bindchips call. updateChips is already there. using it now. pls chk.

@fire-light42 fire-light42 merged commit 91dc83e into recloudstream:master Dec 15, 2023
2 checks passed
@fire-light42
Copy link
Collaborator

👍 All working fine. Nice small pr!

@ghost ghost deleted the search-chip-fix branch December 16, 2023 05:32
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.

None yet

1 participant