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

Supplementary Tracking Issues Take 2 #520

Merged
merged 4 commits into from
Feb 5, 2024

Conversation

kyleve
Copy link
Collaborator

@kyleve kyleve commented Jan 10, 2024

This brings back #433. Please see that PR for the discussion.

Describe your changes here. Please include screenshots if they're visual!

Checklist

Please do the following before merging:

  • Ensure any public-facing changes are reflected in the changelog. Include them in the Main section.

@kyleve kyleve changed the title [WIP DNR] Supplementary Tracking Issues Supplementary Tracking Issues Take 2 Jan 11, 2024
@kyleve kyleve marked this pull request as ready for review January 11, 2024 13:51
@kyleve kyleve requested a review from a team January 11, 2024 13:51
@kyleve
Copy link
Collaborator Author

kyleve commented Jan 11, 2024

Tests are green, should be good to land: https://square-console.sqprod.co/bbr/production/ci_jobs/PR+ios+HJLRR6+Bazel-Pipeline-PR/2

(The one failure is the linter complaining at public github)

@kyleve
Copy link
Collaborator Author

kyleve commented Jan 12, 2024

@n8chur Since you approved the last one, mind stamping this one? The only change is this change 9f3be3a to revert a weak change.

@@ -159,6 +159,7 @@ extension ItemCell {
addGestureRecognizer(panGestureRecognizer)
let tapGestureRecognizer = UITapGestureRecognizer(target: self, action: #selector(handleTap))
tapGestureRecognizer.require(toFail: panGestureRecognizer)
tapGestureRecognizer.cancelsTouchesInView = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does this resolve?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Weird that's showing up here, it's in main #521

Copy link
Collaborator

Choose a reason for hiding this comment

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

huh... I wonder if merging main into this would resolve the confusion GitHub's having here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merging main in seemed to resolve this, odd

* main:
  fix: don't cancel touches in view for tap gesture recognizer
@kyleve kyleve merged commit bc9b064 into main Feb 5, 2024
4 checks passed
@kyleve kyleve deleted the kve/revert-revert-supplementary-tracking branch February 5, 2024 20:12
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.

3 participants