Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Fix for selectedAll field #2226

Merged
merged 2 commits into from
Jan 20, 2022
Merged

Fix for selectedAll field #2226

merged 2 commits into from
Jan 20, 2022

Conversation

nenadV91
Copy link
Contributor

Summary

Fixes #2221

@nenadV91 nenadV91 requested review from a team January 19, 2022 19:59
@github-actions
Copy link
Contributor

  • 🔭 GP Swap: Gnosis Protocol v2 Swap UI

Comment on lines 134 to 136
if (selected.length === paidClaims.length) {
setSelectedAll(true)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be updated when it's not fully selected as well?

Suggested change
if (selected.length === paidClaims.length) {
setSelectedAll(true)
}
setSelectedAll(selected.length === paidClaims.length)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No because if you unselect one option it will set selectedAll to false.

Copy link
Contributor

Choose a reason for hiding this comment

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

...which is what we want 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

Copy link
Contributor

@anxolin anxolin left a comment

Choose a reason for hiding this comment

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

The PR works as expected, although I see a bit of a mess on the responsbilities of the components as discussed in https://gnosisinc.slack.com/archives/C025G521XQD/p1642676703117800?thread_ts=1642674739.113100&cid=C025G521XQD

@nenadV91 nenadV91 requested a review from anxolin January 20, 2022 13:39
@W3stside W3stside merged commit 9367f94 into claim Jan 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants