[No QA] Chore: keep consistent naming in SelectionList and SelectionListWithSections#82839
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Ignore failing ESLint CI check — it warns to add sentry labels to a PressableWithFeedback components, but it doesn't make to add those in a PR that is focused on naming convention refactor mainly. |
| listEmptyContent, | ||
| showScrollIndicator = true, | ||
| showLoadingPlaceholder = false, | ||
| shouldShowLoadingPlaceholder = false, |
There was a problem hiding this comment.
I don't think changing it here makes sense, we'll be removing this file in a week
There was a problem hiding this comment.
@zfurtak I agree, but if since it's gonna be removed anyway, any changes to it doesn't matter that much. Reverting those changes'd be a waste of time as much as applying them.
| canSelectMultiple = false, | ||
| showLoadingPlaceholder = false, | ||
| shouldShowLoadingPlaceholder = false, | ||
| showListEmptyContent = true, |
There was a problem hiding this comment.
What about this props, I think we should change it as well
| showListEmptyContent = true, | |
| shouldShowListEmptyContent = true, |
There was a problem hiding this comment.
@zfurtak absolutely makes sense! Great catch!
b8a75ce to
75c66d9
Compare
| type ButtonOrCheckBoxRoles = 'button' | 'checkbox'; | ||
|
|
||
| type SelectionListHandle<TItem extends ListItem> = { | ||
| type SelectionListHandle<TItem extends ListItem = ListItem> = { |
There was a problem hiding this comment.
@zfurtak by mistake. It's in a commit that is from this PR.
I'll remove it in the original PR and rebase this one.
I originally didn't change SelectionListHandle to SelectionListWithSectionsHandle and added that default type. Later I used the correct type, but forgot about that change! Great catch! 🏅
75c66d9 to
2876842
Compare
…MoneyRequestParticipantsSelector
…SectionList and SectionListWithSections
2876842 to
07a37e5
Compare
Explanation of Change
Use good variable nomenclature in new SelectionList and SelectionListWithSections components.
Per this contributing guide rule:
vars with boolean values should start with prefix 'should'. To be consistent with that rule a prop name was changed as well as var names, which values were passed.
Changes made:
showLoadingPlaceholder- >shouldShowLoadingPlaceholdershowEmptyContent- >shouldShowEmptyContentFixed Issues
$ #65658
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari