Skip to content

fix: remove duplicate bindings#18848

Closed
MorenoTropical wants to merge 1 commit intoankidroid:mainfrom
MorenoTropical:bindings
Closed

fix: remove duplicate bindings#18848
MorenoTropical wants to merge 1 commit intoankidroid:mainfrom
MorenoTropical:bindings

Conversation

@MorenoTropical
Copy link
Contributor

Purpose / Description

Describe the problem or feature and motivation

Fixes

Approach

Instead of combining the bindings, I let it override the binding because I think it is more expected.

How Has This Been Tested?

With an Android 11 emulator, I assigned the same gesture, but with different card sides to see if the side would get overriden.

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

Copy link
Member

@BrayanDSO BrayanDSO left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@BrayanDSO BrayanDSO added the Needs Second Approval Has one approval, one more approval to merge label Jul 12, 2025
Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

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

@BrayanDSO I find the behavior with the new change confusing. If I set the same gesture for answer(after previously setting the question) the text just says: A: Touch Up. Users might think their question binding was removed. I think it should be presented like choosing Question & Answer where it would just say Touch Up.

@BrayanDSO
Copy link
Member

The proposed behavior of overriding makes sense to me. If I already have a gesture assigned to a control and I want to assign it to both sides, I should select Question & Answer. If I'm not selecting that, it's because I want to assign only that side.

But I get how that may be unexpected. The combining behavior may be unexpected as well if I want to assign the control to a single side then remove the other side afterwards.

Maybe the current behavior is the least surprising and we should close this PR. The redundancy issue may not be relevant.

@BrayanDSO BrayanDSO added Needs reviewer reply Waiting for a reply from another reviewer and removed Needs Second Approval Has one approval, one more approval to merge labels Jul 12, 2025
@lukstbit
Copy link
Member

I think the potential confusion after the change(I'm pretty sure this is going to end in a report) outweighs the text duplication. I've also closed the related issue.

Sorry for the wasted effort.

@lukstbit lukstbit closed this Jul 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs reviewer reply Waiting for a reply from another reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Combine redundant controls

3 participants