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

Prevent user from selecting same entry and exit relay for multihop #6455

Conversation

rablador
Copy link
Contributor

@rablador rablador commented Jul 9, 2024

When displaying either the entry or exit location lists, any location (country, city or relay) shouldn't be selectable if, after apply all other constraints, the location item resolves to a single candidate that is already selected by the entry.


This change is Reviewable

@rablador rablador added bug iOS Issues related to iOS labels Jul 9, 2024
@rablador rablador requested review from mojganii and acb-mv July 9, 2024 00:22
Copy link

linear bot commented Jul 9, 2024

@rablador rablador force-pushed the user-shouldnt-be-able-to-select-the-same-entry-location-or-ios-748 branch 2 times, most recently from 859fe47 to 68356f0 Compare July 9, 2024 00:26
Copy link
Contributor Author

@rablador rablador left a comment

Choose a reason for hiding this comment

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

The exclusion is all done in tableView(cellForRowAt). The main reason for this is that the function is very reliable if you want to apply changes during every table update without rebuilding the underlying data set. Since we want to keep the UI state (unfolded cells, position etc) while toggling multihop this is by far the least complex place to "patch" cell contents. We could let the diffable data source handle it for us, but since 1) cell content isn't updated unless the underlying data is changed (which would require adding exclusion state in the nodes, which in turn requires more complexity when populating and manipulating the node tree) and 2) the view state needs to be tracked and rebuilt every time, I opted for the easiest method, knowing that we want to make changes to multihop in the near future anyway.

Reviewable status: 0 of 8 files reviewed, all discussions resolved

Copy link
Contributor Author

@rablador rablador left a comment

Choose a reason for hiding this comment

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

Also, looking at my local git history, Emils' c86bcc3 commit is on top of main. I have rebased from main just now but the commit still wants to be included in this PR for some reason.

Reviewable status: 0 of 8 files reviewed, all discussions resolved

@rablador rablador force-pushed the user-shouldnt-be-able-to-select-the-same-entry-location-or-ios-748 branch from 68356f0 to 9094179 Compare July 9, 2024 00:43
Copy link
Contributor

@acb-mv acb-mv left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: 1 of 7 files reviewed, all discussions resolved

Copy link
Collaborator

@mojganii mojganii left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 1 of 7 files reviewed, all discussions resolved

@pinkisemils pinkisemils force-pushed the user-shouldnt-be-able-to-select-the-same-entry-location-or-ios-748 branch from 9094179 to 5311870 Compare July 11, 2024 11:03
@pinkisemils pinkisemils merged commit f22a319 into main Jul 11, 2024
8 of 9 checks passed
@pinkisemils pinkisemils deleted the user-shouldnt-be-able-to-select-the-same-entry-location-or-ios-748 branch July 11, 2024 11:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug iOS Issues related to iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants