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

[HOLD for payment 2023-07-21] [HOLD for payment 2023-07-20] [Accessible Pressables] Add new ESLint rules to enforce use of accessible pressable components #17004

Closed
6 tasks
roryabraham opened this issue Apr 6, 2023 · 17 comments
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 NewFeature Something to build that is a new item.

Comments

@roryabraham roryabraham added the NewFeature Something to build that is a new item. label Apr 6, 2023
@roryabraham roryabraham self-assigned this Apr 6, 2023
@MelvinBot

This comment was marked as resolved.

@melvin-bot melvin-bot bot added the Weekly KSv2 label Apr 6, 2023
@roryabraham roryabraham added Monthly KSv2 and removed Weekly KSv2 labels Apr 6, 2023
@roryabraham
Copy link
Contributor Author

cc @robertKozik

@roryabraham roryabraham removed their assignment Apr 6, 2023
@roryabraham roryabraham moved this from HOLD to Todo in Accessible Pressables May 11, 2023
@roryabraham roryabraham changed the title [HOLD][Accessible Pressables] Add new ESLint rules to enforce use of accessible pressable components [Accessible Pressables] Add new ESLint rules to enforce use of accessible pressable components May 11, 2023
@melvin-bot melvin-bot bot added the Overdue label May 11, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 19, 2023

@roryabraham, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@melvin-bot melvin-bot bot closed this as completed Jun 19, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Accessible Pressables Jun 19, 2023
@robertKozik
Copy link
Contributor

robertKozik commented Jun 19, 2023

@roryabraham There is still an unmerged PR (draft to be precise) from @Skalakid, looks like project automation close it sooner as it should
edit: looks like other issues from this project suffered the same fate

@roryabraham
Copy link
Contributor Author

roryabraham commented Jun 22, 2023

@Skalakid do you think the migration's at the point where we can disable new non-accessible pressables going forward?

@Skalakid
Copy link
Contributor

@roryabraham I have one more PR to be merged, but I think yes. I will open my PR #20363

@Skalakid
Copy link
Contributor

Skalakid commented Jun 23, 2023

@roryabraham Okay, so I fixed all ESlint errors connected with accessibility. While doing it I found out that there are a few new components that are using "old" Pressables:

  • SignInPageGraphics
  • ThreePaneView
  • FloatingActionButton
  • Badge

I think we can move forward with the PR! What do you think bout it @robertKozik?

@robertKozik
Copy link
Contributor

Yeah, we need to cut the flow of the new Pressables. The amount of them is bearable for now, so I think we should prohibit the usage of react-native Pressable ASAP and then migrate the last of them.
CC. @roryabraham

@melvin-bot melvin-bot bot added the Reviewing Has a PR in review label Jun 26, 2023
@Skalakid
Copy link
Contributor

Skalakid commented Jun 26, 2023

Hi @roryabraham, I just found a bug in our ESlint rules. We have 3 no-restricted-imports rules for react-native but with different messages. Unfortunately, only the last one in the paths array is enabled. It is a known issue and ESlint is working on it, you can find it here eslint/eslint#15261.

To make this PR work I moved the Pressable rule to the bottom, but this is only a workaround. Should I fix this issue in my PR? I think it can be solved by adding custom eslint rules

@blazejkustra
Copy link
Contributor

@Skalakid @roryabraham I also noticed this bug in Setup App for TypeScript #20179. My workaround for now is to group imports together with a longer description:

{
    name: 'react-native',
    importNames: ['useWindowDimensions', 'TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight', 'StatusBar'],
    message: [
        "For 'useWindowDimensions', please use 'src/hooks/useWindowDimensions' instead.",
        "For 'TouchableOpacity', 'TouchableWithoutFeedback', 'TouchableNativeFeedback', 'TouchableHighlight', please use 'PressableWithFeedback' and/or 'PressableWithoutFeedback' from 'src/components/Pressable' instead.",
        "For 'StatusBar', please use 'src/libs/StatusBar' instead.",
    ].join('\n'),
},

This way all listed react-native imports are restricted.

@roryabraham
Copy link
Contributor Author

Thanks @blazejkustra. I've hired someone to implement that workaround (they beat you to it on #22459)

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Monthly KSv2 labels Jul 13, 2023
@melvin-bot melvin-bot bot changed the title [Accessible Pressables] Add new ESLint rules to enforce use of accessible pressable components [HOLD for payment 2023-07-20] [Accessible Pressables] Add new ESLint rules to enforce use of accessible pressable components Jul 13, 2023
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jul 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 13, 2023

Reviewing label has been removed, please complete the "BugZero Checklist".

@melvin-bot
Copy link

melvin-bot bot commented Jul 13, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.39-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-07-20. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jul 14, 2023
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2023-07-20] [Accessible Pressables] Add new ESLint rules to enforce use of accessible pressable components [HOLD for payment 2023-07-21] [HOLD for payment 2023-07-20] [Accessible Pressables] Add new ESLint rules to enforce use of accessible pressable components Jul 14, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 14, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.40-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-07-21. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jul 19, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 27, 2023

6 days overdue. This is scarier than being forced to listen to Vogon poetry!

@melvin-bot melvin-bot bot added the Overdue label Jul 27, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 1, 2023

12 days overdue. Walking. Toward. The. Light...

@roryabraham
Copy link
Contributor Author

This is done

@melvin-bot melvin-bot bot removed the Overdue label Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 NewFeature Something to build that is a new item.
Projects
No open projects
Status: Done
Development

No branches or pull requests

6 participants