[RNMobile] Performance improvements in File related controls #28224
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
|
Size Change: 0 B Total Size: 1.37 MB ℹ️ View Unchanged
|
lukewalczak
added
[Type] Performance
dratwas
reviewed
Jan 25, 2021
lukewalczak
requested review from
ajitbohra and
chrisvanpatten
as code owners
dratwas
approved these changes
Jan 28, 2021
|
@lukewalczak while working on unrelated work, I noticed that this PR removes the I presume this likely an oversight of some kind. Do you know if the intention was to keep the lowered opacity for this button when it's disabled or was there a different styling that should be applied? Either way, we likely need to fix/remove the lingering reference to a non-existing class name. Reference Image
Once I understand the desired outcome, I'm happy to help create a new issue to track the work. Thanks! |
Hi @dcalhoun I just happened to see this comment. I worked on the File Block so I can confirm that the opacity should be lowered when the Download button is disabled. One of the states in which it gets disabled is during the file upload process. |
|
Looks like the oversight from my side. If you can please create an issue for that and I will handle it asap to have the fix upcoming release ✌🏽 Thank you! |
6 tasks
|
Created a PR which moves back the style. Could you please @jd-alexander or @dcalhoun do a testing and reviewing? |
|
@jd-alexander @lukewalczak thanks for the info. #28706 captures the issue. I will check out the PR branch shortly. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Merge after
VideoPR: #28220Description
Part of: #28196
Contains changes only related to the
Fileblock and controls available within block settings and inherit changes fromVideoPR.SelectControlToggleControlToggleControlTextControlHow has this been tested?
Test manually
Fileblock on web and mobile.You can paste the snippet to have
Fileblock filled:or
Fileblock and attach fileExpect everything works correctly
Screenshots
RESULTS
1. pressing copy url button
Types of changes
Performance improvements
Checklist: