Addressed review
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
Feed All Stories
All Stories
All Stories
May 18 2023
May 18 2023
In D7793#232996, @michal wrote:Accepting, but please fix the icon before landing!
Address review
Not a fan of this from an accessibility stand point. I found something like this: focus-trap, we could wrap all our modals with it. But I'm not sure if that's worth it. @ashoat any thoughts?
Accepting, but please fix the icon before landing!
May 17 2023
May 17 2023
Requesting changes only to get answer about using comm-services-lib
Confused about the most recent revision
I should have been more specific
Merged with D7824
Just a few more points about the buttons, otherwise looks good!
Addressed review
All other icons are filled in.
I can see that on figma that all buttons are outlines and there's a bar between reactions and the rest of the buttons. Not sure if it will be handled on some other occasion or if there's some more context on this, but I feel like they should be at least consistent.
It's true that diffs should be small, but that is just an "instrumental goal". The real goal is to make diffs readable and clear to reviewers, and usually small diffs are more readable and clear. But that's not always the case... often, splitting a diff into two can make it harder to review, especially if the two parts depend closely on each other. In this case I agree with @michal that it would be easier to review if D7824 and this diff were merged.
As far as I know, diffs should be small, and do only one thing. The division between those diffs is simple: one diff adds the button itself to the tooltip, and the other adds its functionality (entering edit state).
Also, this diff cannot depend on previous diffs, because it is first in the stack.
ashoat added inline comments to D7847: [web] Call `create_thread` from `CommunityCreationModal` on form submission.
ashoat committed rCOMM1710139fc7b7: [native] Add loading spinner to RegistrationButton loading variant (authored by ashoat).
[native] Add loading spinner to RegistrationButton loading variant
ashoat committed rCOMMf1983bafc7eb: [native] Introduce RegistrationContainer to handle SafeAreaView (authored by ashoat).
[native] Introduce RegistrationContainer to handle SafeAreaView
ashoat committed rCOMMed64f3949075: [native] Add SIWEPanel to ConnectEthereum screen (authored by ashoat).
[native] Add SIWEPanel to ConnectEthereum screen
ashoat committed rCOMMf8b83d7961d2: [native] Split onClosing and onClosed params to SIWEPanel (authored by ashoat).
[native] Split onClosing and onClosed params to SIWEPanel
In D7814#232863, @marcin wrote:Handle aps dictionary.
Correctly calculate whether messageInfos should be included into notification payload
Handle aps dictionary.
marcin updated the diff for D7813: Implement function to encrypt relevant parts of iOS notification.
Create new notification instead of mutating the existing one
Rebase
Rebase
bartek committed rCOMM676e53b30a78: [keyserver] Add thumbhash to upload endpoints (authored by bartek).
[keyserver] Add thumbhash to upload endpoints
bartek committed rCOMMcd377781d242: Add thumbhash field to multimedia messages (authored by bartek).
Add thumbhash field to multimedia messages
bartek committed rCOMM201e05a269b8: [native] Avoid transition delay during upload (authored by bartek).
[native] Avoid transition delay during upload
bartek committed rCOMMb8b133037c37: [native] Add thumbhash step to media mission (authored by bartek).
[native] Add thumbhash step to media mission
bartek committed rCOMM253f5590e279: [native] Implement thumbhash generation on Android (authored by bartek).
[native] Implement thumbhash generation on Android
bartek committed rCOMM991a876865aa: [native] Implement thumbhash generation on iOS (authored by bartek).
[native] Implement thumbhash generation on iOS
bartek committed rCOMMc768750741aa: [native] Add thumbHash prop to image components (authored by bartek).
[native] Add thumbHash prop to image components
bartek committed rCOMM23665e51b022: [native] Create expo-module for Thumbhash generation (authored by bartek).
[native] Create expo-module for Thumbhash generation
bartek committed rCOMMb1ecdd8e60c5: [native] Add utils to encrypt/decrypt base64 string (authored by bartek).
[native] Add utils to encrypt/decrypt base64 string
bartek committed rCOMMde5bbb31d640: [native] Add base64 decode to CommUtilsModule (authored by bartek).
[native] Add base64 decode to CommUtilsModule
bartek committed rCOMM0d7e1448bd31: [native] Add C++ function to decode base64 (authored by bartek).
[native] Add C++ function to decode base64
In D7789#232531, @ashoat wrote:I was asking for links (hyperlinks) to the TypeScript types. In a previous comment I had requested:
Can you find the TypeScript types and try to base your new types on those, and make sure to include all relevant params?
Assuming you did that and found the TypeScript types, I'm now asking you to link those types. With the comments you left, it appears that some of them don't match, and I can't tell if they are TypeScript types or just other Flow types that we already have.
Rebase, validate uploadMediaMetadataResponder, fix bug with uploadDownloadResponder
Clearing tooltip after entering edit mode
Rebase
Rebase
Rebase
Rebase
Rebase
Rebase
Rebase
Rebase