Add indicator to avatar#10065
Conversation
dc9ac98 to
29ac710
Compare
f8051a3 to
78fa853
Compare
|
@iwiznia can you add some testing steps? please |
|
Yeah sorry, sent this in late yesterday and wanted an early review (expecting comments and changes anyway). Doing that now. This is again a hard to test PR since the backend is not ready and so manual things are needed. |
| styles.justifyContentCenter, | ||
| props.size === 'large' ? styles.statusIndicatorLarge : styles.statusIndicator, | ||
| ]; | ||
| const isLarge = props.size === 'large'; |
There was a problem hiding this comment.
move const isLarge = props.size === 'large'; before const indicatorStyles and use it in props.size === 'large' ? styles.statusIndicatorLarge : styles.statusIndicator,
thienlnam
left a comment
There was a problem hiding this comment.
did an initial review, looks good so far - a few questions, will item.login change?
| {this.props.isSyncing && ( | ||
| <Icon | ||
| src={Expensicons.Sync} | ||
| fill={themeColors.textReversed} | ||
| width={6} | ||
| height={6} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Are we purposely removing the sync icon?
| isLarge ? styles.statusIndicatorLarge : styles.statusIndicator, | ||
| ]; | ||
|
|
||
| const hasError = _.some(props.policiesMemberList, policyMembers => hasPolicyMemberError(policyMembers)); |
There was a problem hiding this comment.
NAB: hasPolicyMemberError
|
|
||
| /** Is this action pending? */ | ||
| pending: PropTypes.string, |
There was a problem hiding this comment.
Do we also need pendingAction?
There was a problem hiding this comment.
Oh crap, yes, this is wrong.
Probably... |
|
Seems @aldo-expensify is OOO, so merging as is. If there are additional comments, I can address those in another PR |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to production by @yuwenmemon in version: 1.1.87-9 🚀
|
Details
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/218719
Tests
This needs manually changing data/code to test (since the API is not done yet):
ie: window.Onyx = Onyxon a file that imports OnyxOnyx.set('policyMemberList_2D0BDD1B7A684C67', {"ionatan@expensify.com": {pending: 'add', errors: {1: 'This is an error'}}})
- Check the policy you set the error in has a red dot
- Check manage members of that policy (but not others) has the red dot
- [Here](https://github.com/Expensify/App/blob/983d1922b2fd3e8a9e4359df7bba220a2d9e6753/src/pages/workspace/WorkspaceMembersPage.js#L272) add an error and pendingAction: ie: `.map(person => ({...person, pendingAction: 'delete', errors: {1: 'caca'}}))` - Check the member has the error displayed
- Note that dismissing the error is not working now, since the error is hardcoded in the code abovePR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)PR Reviewer Checklist
The Contributor+ will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issuessection aboveTestssectionQA stepssectiontoggleReportand notonIconClick).src/languages/*filesSTYLE.md) were followedAvatar, I verified the components usingAvatarhave been tested & I retested again)/** comment above it */displayNamepropertythisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)Avataris modified, I verified thatAvataris working as expected in all cases)QA Steps
Screenshots
Web
Mobile Web
Desktop
iOS
Android