Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
2.99k stars 2.5k forks source link

Add import text input clear button #41661

Closed nkdengineer closed 1 week ago

nkdengineer commented 1 week ago

Details

Fixed Issues

$ https://github.com/Expensify/App/issues/40313 PROPOSAL:

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
melvin-bot[bot] commented 1 week ago

Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers!

melvin-bot[bot] commented 1 week ago

@dannymcclain @eh2077 One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

nkdengineer commented 1 week ago

In the previous PR I was missing some imports Please check again @eh2077 @marcaaron

eh2077 commented 1 week ago

That's weird that this wasn't caught by CI - Lint code / Run ESLint (pull_request). I believe @marcaaron should have checked All checks have passed before merging.

eh2077 commented 1 week ago

I saw the last lint check of the previous PR was passed

image

Click this link to see lint check records https://github.com/Expensify/App/actions/workflows/lint.yml?query=actor%3Ankdengineer+branch%3Afix%2F40313

eh2077 commented 1 week ago

@nkdengineer Can you help to check locally if npm run lint can catch these missing imports?

nkdengineer commented 1 week ago

@eh2077 in my locally npm run lint can't catch these missing imports

eh2077 commented 1 week ago

@eh2077 in my locally npm run lint can't catch these missing imports

Ok. npm run typecheck also can't catch this, see https://github.com/Expensify/App/actions/workflows/typecheck.yml?query=actor%3Ankdengineer+branch%3Afix%2F40313

I think this should be a potential improvements for lint or typecheck or both

eh2077 commented 1 week ago

Reviewer Checklist

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari Screenshot 2024-05-06 at 8 04 25β€―AM
MacOS: Desktop
OSBotify commented 1 week ago

πŸš€ Deployed to production by https://github.com/marcaaron in version: 1.4.71-6 πŸš€

platform result
πŸ€– android πŸ€– success βœ…
πŸ–₯ desktop πŸ–₯ success βœ…
🍎 iOS 🍎 success βœ…
πŸ•Έ web πŸ•Έ success βœ