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

Address WPAndroid ClickableViewAccessibility lint errors #706

Merged
merged 3 commits into from
Jun 25, 2021

Conversation

aforcier
Copy link
Collaborator

It looks like the view binding update (#688) created a few cases of the ClickableViewAccessibility lint check. Nothing changed feature-wise, I'm guessing the check just parses view binding based code differently and recognized something where it didn't before. I noticed this in wordpress-mobile/WordPress-Android#14928, which is trying to pull the view binding changes from this library into WPAndroid.

ClickableViewAccessibility is a warning by default, but WPAndroid has promoted this to an error.

We have an open issue (#499) to eventually try to address some ClickableViewAccessiblity suppressions we already have, which I believe were imported with the original photoeditor library. (It may in fact be a true false positive in some cases due to how we're handling clicks.)

For now, to unblock that PR, I'm suppressing the lint check in the areas where it's now being flagged. I've also imported the lint.xml file from WPAndroid to make this less likely to happen in the future (I removed a few obvious WPAndroid-specific things). Ideally this will be part of better tooling in the future allowing us to share the lint.xml between projects with local custom rules.

To test

  • Check out ce3fe2f, run lint, confirm you get 11 errors (same as the WPAndroid PR)
  • Check out the branch, run lint, confirm no errors
  • The above two should also be verifiable by looking at CI, I'm pushing the PR early on purpose so CI runs lint on ce3fe2f
  • Build this branch in WPAndroid, run lintWordpressVanillaRelease, make sure there are no errors

@peril-automattic
Copy link

peril-automattic bot commented Jun 25, 2021

You can test the changes on this Pull Request by downloading the APK here.

@malinajirka
Copy link
Contributor

Works as expected, thanks! 🙇

@malinajirka malinajirka merged commit f110ad8 into develop Jun 25, 2021
@malinajirka malinajirka deleted the fix/wpandroid-lint-errors branch June 25, 2021 13:41
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.

2 participants