Page MenuHomePhabricator
Differential D7498

[native] Pull `useSelectAndUploadFromGallery` out of `Edit[User/Thread]Avatar`
ClosedPublic
Actions

Authored by atul on Apr 18 2023, 12:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 12, 3:21 PM
Unknown Object (File)
Thu, Mar 6, 9:40 PM
Unknown Object (File)
Wed, Mar 5, 2:58 AM
Unknown Object (File)
Mon, Mar 3, 4:44 PM
Unknown Object (File)
Mon, Mar 3, 4:44 PM
Unknown Object (File)
Mon, Mar 3, 4:44 PM
Unknown Object (File)
Sun, Mar 2, 7:35 AM
Unknown Object (File)
Sun, Mar 2, 7:35 AM
View All 85 Files
Subscribers
tomek

Details

Reviewers
ashoat
ginsu
Commits
rCOMMefe9eb49757d: [native] Pull `useSelectAndUploadFromGallery` out of `Edit[User/Thread]Avatar`
Summary

Pull out common functionality from Edit[User/Thread]Avatar and move to newly introduced avatar-hooks.js.

Added entry to editAvatarOptions in both components to consume this hook.

Depends on D7497

Test Plan
  1. Click on pencil icon
  2. Ensure that the system media gallery appears as expected
  3. Ensure that the selection is processed as expected (logging + breakpoint)
  4. Ensure that selected image appears in keyserver uploads table:

1d60c3.png (1×3 px, 2 MB)

c40a6d.png (1×3 px, 1 MB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul created this revision.Apr 18 2023, 12:27 PM
Herald added a subscriber: tomek. · View Herald TranscriptApr 18 2023, 12:27 PM
atul published this revision for review.Apr 18 2023, 12:28 PM
atul edited the test plan for this revision. (Show Details)
atul added inline comments.
native/avatars/edit-user-avatar.react.js
25 ↗(On Diff #25298)

For now useSelectAndUploadFromGallery just returns the function that launches that media gallery and processes + uploads the selection. In the future it'll probably also return some information about progress.

atul added inline comments.Apr 18 2023, 12:42 PM
native/avatars/edit-thread-avatar.react.js
46–50 ↗(On Diff #25298)

Here's how things looks on Android. I think we should use emote-smile for consistency for emoji

c8ad87.png (244×834 px, 26 KB)

atul attached a referenced file: F494242: c8ad87.png. (Show Details)Apr 18 2023, 12:42 PM
Harbormaster completed remote builds in B18472: Diff 25298.Apr 18 2023, 12:45 PM
atul added a child revision: D7499: [native] Use `outline` variant of `emote-smile` in `Edit*Avatar` for consistency.Apr 18 2023, 1:08 PM
ashoat accepted this revision.Apr 18 2023, 1:14 PM

Assuming this is a clean refactor so only skimmed it

native/avatars/edit-thread-avatar.react.js
46–50 ↗(On Diff #25298)

What does emote-smile look like? Agree this doesn't look great

native/avatars/edit-user-avatar.react.js
25 ↗(On Diff #25298)

Makes sense. Not sure if the "information about progress" will be returned by the call to useSelectAndUploadFromGallery() or the call to selectAndUploadFromGallery(), but I think the latter makes more sense since there's no progress to speak of in the former

41–52 ↗(On Diff #25298)

Thank you for adding this!

This revision is now accepted and ready to land.Apr 18 2023, 1:14 PM
atul added inline comments.Apr 18 2023, 1:18 PM
native/avatars/edit-thread-avatar.react.js
46–50 ↗(On Diff #25298)

Have before and after in D7499

Closed by commit rCOMMefe9eb49757d: [native] Pull `useSelectAndUploadFromGallery` out of `Edit[User/Thread]Avatar` (authored by atul). · Explain WhyApr 18 2023, 1:20 PM
This revision was automatically updated to reflect the committed changes.
atul added a commit: rCOMMefe9eb49757d: [native] Pull `useSelectAndUploadFromGallery` out of `Edit[User/Thread]Avatar`.

Revision Contents
Changeset List

PathSize
native/
avatars/
64 lines
81 lines
81 lines

Diff 25307

View Options

native/avatars/avatar-hooks.js

Show First 20 Lines • Show All 135 Lines • ▼ Show 20 Linesconst light = Object.freeze({
subthreadsModalClose: designSystemColors.shadesBlack60, subthreadsModalClose: designSystemColors.shadesBlack60,
subthreadsModalBackground: designSystemColors.shadesWhite80, subthreadsModalBackground: designSystemColors.shadesWhite80,
subthreadsModalSearch: '#00000008', subthreadsModalSearch: '#00000008',
messageLabel: designSystemColors.shadesBlack100, messageLabel: designSystemColors.shadesBlack100,
modalSeparator: designSystemColors.shadesWhite60, modalSeparator: designSystemColors.shadesWhite60,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesBlack100, inviteLinkLinkColor: designSystemColors.shadesBlack100,
inviteLinkButtonBackground: designSystemColors.shadesWhite60, inviteLinkButtonBackground: designSystemColors.shadesWhite60,
emojiKeyboardKnob: designSystemColors.shadesWhite90,
emojiKeyboardCategoryIcon: designSystemColors.shadesBlack70,
emojiKeyboardContainerActive: designSystemColors.shadesWhite90,
ashoatUnsubmitted
Not Done
Inline Actions

Do we really need one-offs here? Why can't we find an existing color family that works with our designs?

We need to push the team (@tedchang in particular) to use existing families (and update them as necessary) instead of hacking around inconsistencies like this. When we take this sort of approach of picking colors on a per-screen basis, we fail to maintain stylistic / design consistency throughout the app

ashoat: Do we really need one-offs here? Why can't we find an existing color family that works with our…
ashoatUnsubmitted
Not Done
Inline Actions

Meant to tag @ted

ashoat: Meant to tag @ted
}); });
export type Colors = $Exact<typeof light>; export type Colors = $Exact<typeof light>;
const dark: Colors = Object.freeze({ const dark: Colors = Object.freeze({
blockQuoteBackground: '#A9A9A9', blockQuoteBackground: '#A9A9A9',
blockQuoteBorder: designSystemColors.shadesBlack60, blockQuoteBorder: designSystemColors.shadesBlack60,
codeBackground: designSystemColors.shadesBlack100, codeBackground: designSystemColors.shadesBlack100,
disabledButton: designSystemColors.shadesBlack80, disabledButton: designSystemColors.shadesBlack80,
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Linesconst dark: Colors = Object.freeze({
typeaheadTooltipBackground: '#1F1F1f', typeaheadTooltipBackground: '#1F1F1f',
typeaheadTooltipBorder: designSystemColors.shadesBlack80, typeaheadTooltipBorder: designSystemColors.shadesBlack80,
typeaheadTooltipText: 'white', typeaheadTooltipText: 'white',
messageLabel: designSystemColors.shadesWhite60, messageLabel: designSystemColors.shadesWhite60,
modalSeparator: designSystemColors.shadesBlack80, modalSeparator: designSystemColors.shadesBlack80,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesWhite80, inviteLinkLinkColor: designSystemColors.shadesWhite80,
inviteLinkButtonBackground: designSystemColors.shadesBlack80, inviteLinkButtonBackground: designSystemColors.shadesBlack80,
emojiKeyboardKnob: designSystemColors.shadesWhite100,
emojiKeyboardCategoryIcon: designSystemColors.shadesWhite70,
emojiKeyboardContainerActive: designSystemColors.shadesBlack60,
}); });
const colors = { light, dark }; const colors = { light, dark };
const colorsSelector: (state: AppState) => Colors = createSelector( const colorsSelector: (state: AppState) => Colors = createSelector(
(state: AppState) => state.globalThemeInfo.activeTheme, (state: AppState) => state.globalThemeInfo.activeTheme,
(theme: ?GlobalTheme) => { (theme: ?GlobalTheme) => {
const explicitTheme = theme ? theme : 'light'; const explicitTheme = theme ? theme : 'light';
return colors[explicitTheme]; return colors[explicitTheme];
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines
View Options

native/avatars/edit-thread-avatar.react.js

Show First 20 Lines • Show All 135 Lines • ▼ Show 20 Linesconst light = Object.freeze({
subthreadsModalClose: designSystemColors.shadesBlack60, subthreadsModalClose: designSystemColors.shadesBlack60,
subthreadsModalBackground: designSystemColors.shadesWhite80, subthreadsModalBackground: designSystemColors.shadesWhite80,
subthreadsModalSearch: '#00000008', subthreadsModalSearch: '#00000008',
messageLabel: designSystemColors.shadesBlack100, messageLabel: designSystemColors.shadesBlack100,
modalSeparator: designSystemColors.shadesWhite60, modalSeparator: designSystemColors.shadesWhite60,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesBlack100, inviteLinkLinkColor: designSystemColors.shadesBlack100,
inviteLinkButtonBackground: designSystemColors.shadesWhite60, inviteLinkButtonBackground: designSystemColors.shadesWhite60,
emojiKeyboardKnob: designSystemColors.shadesWhite90,
emojiKeyboardCategoryIcon: designSystemColors.shadesBlack70,
emojiKeyboardContainerActive: designSystemColors.shadesWhite90,
ashoatUnsubmitted
Not Done
Inline Actions

Do we really need one-offs here? Why can't we find an existing color family that works with our designs?

We need to push the team (@tedchang in particular) to use existing families (and update them as necessary) instead of hacking around inconsistencies like this. When we take this sort of approach of picking colors on a per-screen basis, we fail to maintain stylistic / design consistency throughout the app

ashoat: Do we really need one-offs here? Why can't we find an existing color family that works with our…
ashoatUnsubmitted
Not Done
Inline Actions

Meant to tag @ted

ashoat: Meant to tag @ted
}); });
export type Colors = $Exact<typeof light>; export type Colors = $Exact<typeof light>;
const dark: Colors = Object.freeze({ const dark: Colors = Object.freeze({
blockQuoteBackground: '#A9A9A9', blockQuoteBackground: '#A9A9A9',
blockQuoteBorder: designSystemColors.shadesBlack60, blockQuoteBorder: designSystemColors.shadesBlack60,
codeBackground: designSystemColors.shadesBlack100, codeBackground: designSystemColors.shadesBlack100,
disabledButton: designSystemColors.shadesBlack80, disabledButton: designSystemColors.shadesBlack80,
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Linesconst dark: Colors = Object.freeze({
typeaheadTooltipBackground: '#1F1F1f', typeaheadTooltipBackground: '#1F1F1f',
typeaheadTooltipBorder: designSystemColors.shadesBlack80, typeaheadTooltipBorder: designSystemColors.shadesBlack80,
typeaheadTooltipText: 'white', typeaheadTooltipText: 'white',
messageLabel: designSystemColors.shadesWhite60, messageLabel: designSystemColors.shadesWhite60,
modalSeparator: designSystemColors.shadesBlack80, modalSeparator: designSystemColors.shadesBlack80,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesWhite80, inviteLinkLinkColor: designSystemColors.shadesWhite80,
inviteLinkButtonBackground: designSystemColors.shadesBlack80, inviteLinkButtonBackground: designSystemColors.shadesBlack80,
emojiKeyboardKnob: designSystemColors.shadesWhite100,
emojiKeyboardCategoryIcon: designSystemColors.shadesWhite70,
emojiKeyboardContainerActive: designSystemColors.shadesBlack60,
}); });
const colors = { light, dark }; const colors = { light, dark };
const colorsSelector: (state: AppState) => Colors = createSelector( const colorsSelector: (state: AppState) => Colors = createSelector(
(state: AppState) => state.globalThemeInfo.activeTheme, (state: AppState) => state.globalThemeInfo.activeTheme,
(theme: ?GlobalTheme) => { (theme: ?GlobalTheme) => {
const explicitTheme = theme ? theme : 'light'; const explicitTheme = theme ? theme : 'light';
return colors[explicitTheme]; return colors[explicitTheme];
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines
View Options

native/avatars/edit-user-avatar.react.js

Show First 20 Lines • Show All 135 Lines • ▼ Show 20 Linesconst light = Object.freeze({
subthreadsModalClose: designSystemColors.shadesBlack60, subthreadsModalClose: designSystemColors.shadesBlack60,
subthreadsModalBackground: designSystemColors.shadesWhite80, subthreadsModalBackground: designSystemColors.shadesWhite80,
subthreadsModalSearch: '#00000008', subthreadsModalSearch: '#00000008',
messageLabel: designSystemColors.shadesBlack100, messageLabel: designSystemColors.shadesBlack100,
modalSeparator: designSystemColors.shadesWhite60, modalSeparator: designSystemColors.shadesWhite60,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesBlack100, inviteLinkLinkColor: designSystemColors.shadesBlack100,
inviteLinkButtonBackground: designSystemColors.shadesWhite60, inviteLinkButtonBackground: designSystemColors.shadesWhite60,
emojiKeyboardKnob: designSystemColors.shadesWhite90,
emojiKeyboardCategoryIcon: designSystemColors.shadesBlack70,
emojiKeyboardContainerActive: designSystemColors.shadesWhite90,
ashoatUnsubmitted
Not Done
Inline Actions

Do we really need one-offs here? Why can't we find an existing color family that works with our designs?

We need to push the team (@tedchang in particular) to use existing families (and update them as necessary) instead of hacking around inconsistencies like this. When we take this sort of approach of picking colors on a per-screen basis, we fail to maintain stylistic / design consistency throughout the app

ashoat: Do we really need one-offs here? Why can't we find an existing color family that works with our…
ashoatUnsubmitted
Not Done
Inline Actions

Meant to tag @ted

ashoat: Meant to tag @ted
}); });
export type Colors = $Exact<typeof light>; export type Colors = $Exact<typeof light>;
const dark: Colors = Object.freeze({ const dark: Colors = Object.freeze({
blockQuoteBackground: '#A9A9A9', blockQuoteBackground: '#A9A9A9',
blockQuoteBorder: designSystemColors.shadesBlack60, blockQuoteBorder: designSystemColors.shadesBlack60,
codeBackground: designSystemColors.shadesBlack100, codeBackground: designSystemColors.shadesBlack100,
disabledButton: designSystemColors.shadesBlack80, disabledButton: designSystemColors.shadesBlack80,
▲ Show 20 Lines • Show All 81 Lines • ▼ Show 20 Linesconst dark: Colors = Object.freeze({
typeaheadTooltipBackground: '#1F1F1f', typeaheadTooltipBackground: '#1F1F1f',
typeaheadTooltipBorder: designSystemColors.shadesBlack80, typeaheadTooltipBorder: designSystemColors.shadesBlack80,
typeaheadTooltipText: 'white', typeaheadTooltipText: 'white',
messageLabel: designSystemColors.shadesWhite60, messageLabel: designSystemColors.shadesWhite60,
modalSeparator: designSystemColors.shadesBlack80, modalSeparator: designSystemColors.shadesBlack80,
secondaryButtonBorder: designSystemColors.shadesWhite100, secondaryButtonBorder: designSystemColors.shadesWhite100,
inviteLinkLinkColor: designSystemColors.shadesWhite80, inviteLinkLinkColor: designSystemColors.shadesWhite80,
inviteLinkButtonBackground: designSystemColors.shadesBlack80, inviteLinkButtonBackground: designSystemColors.shadesBlack80,
emojiKeyboardKnob: designSystemColors.shadesWhite100,
emojiKeyboardCategoryIcon: designSystemColors.shadesWhite70,
emojiKeyboardContainerActive: designSystemColors.shadesBlack60,
}); });
const colors = { light, dark }; const colors = { light, dark };
const colorsSelector: (state: AppState) => Colors = createSelector( const colorsSelector: (state: AppState) => Colors = createSelector(
(state: AppState) => state.globalThemeInfo.activeTheme, (state: AppState) => state.globalThemeInfo.activeTheme,
(theme: ?GlobalTheme) => { (theme: ?GlobalTheme) => {
const explicitTheme = theme ? theme : 'light'; const explicitTheme = theme ? theme : 'light';
return colors[explicitTheme]; return colors[explicitTheme];
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines