- Make AndroidNotification type read-only
- Change order of assignments to increase readability
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
May 24 2023
Shorthand
Make props read only
Rebase
Address review
Looks good to me but I'd also let @varun take a look
In D7730#235617, @ashoat wrote:As @ted is off for now, I don't think it makes sense to block this diff on this change.
Can you create a DES task and assign it to Ted, to make sure we follow up after he is back?
Reworked the diff to the new approach. Now encrypted media also use LoadableVideo to display thumbhash and thumbnail for the video.
Reworked the diff to the new approach. The only significant change here is that one usage of the state defined in this diff is moved to D7900.
Reworked the video approach to display actual thumbnails as soon as possible. I admit this looks well, but needed to throttle network to "Slow 3G" to be able to see the effect in some cases. I'm not sure if this is a good idea, but I think it's better than nothing. I also added a placeholder for the thumbnail in case the video is not loaded yet.
Now it works especially as requested: The thumbnail is displayed as soon as it is downloaded/decrypted and replaces the thumbhash. Code for this is in D7946
- Changed logic to display actual video thumbnail as soon as possible
- Added support for encrypted video thumbnails
- Added a forwardRef to the video element which will be needed for encrypted media
In D7902#235707, @ashoat wrote:Whereas my view is that the thumbhash is just meant to make the transition of loading the content more "seamless", but otherwise the behavior should be to aim to present the highest fidelity version of the content we can display at any given moment.
In terms of concrete benefits, presenting the highest fidelity version of the content allows the user to "see" things as quickly as possible, which I think improves the user experience.
May 23 2023
fix import
The placeholder color you're using is super bright. Can you link to designs? Feels like it might be best to stick with what RegistrationTextInput does for consistency
High-level approach is great, but some comments below
Passing back to your queue, but feel free to-request review again if there's something that I'm missing. Also please let me know if this would be a difficult change to make... my impression is that changes in D7946 should hopefully be easy, but I'm not sure about the whole scope of changes that would be required.
I think either there's a disconnect about when to display the thumbhash vs. the thumbnail, or perhaps I am misunderstanding the feasibility of my suggestion. Please let me know if I'm missing something!
Thanks for explaining everything!
The last comment was pretty concerning...
As @ted is off for now, I don't think it makes sense to block this diff on this change.
Assuming @ashoat has signed off on copy + we're fine with having lines extend past 80 characters since this is a "data" file vs. "code" file?
address comments and remove logoImageURL field (since we are using svgs now)
address comments