Repository navigation
Separate theme logic - #117818
Separate theme logic#117818
Conversation
flutter-dashboard
Bot
added
p: material_ui
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
Why? |
imo it makes it more readable by separating the tree view from details, as someone who had never read it before seeing this PR I'd say say it's better than before. |
1f6c2c7 to
0c08f4c
Compare
|
I have no idea why Windows build_tests_3_3 failed 😞 |
|
test-exempt: code refactor with no semantic change (this should not influence the reviewers' decision about whether this is a good idea or not; in particular, this change might have an imperceptible performance cost since it adds another method call, so if the readability benefit is not substantial then maybe we should explore other ways to improve the readability) |
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
Sorry that it has taken so long to get this PR reviewed! This is a small change and I think the title may have scared off the original reviewer.
The changes you're proposing seem reasonable. If you'd be willing to sync your PR up, I think we could land this.
|
@HansMuller Thanks for the review! I will sync it later. |
|
@HansMuller It's done! 🚀 |
🐔 |
d383fa0 to
ff381fc
Compare
HansMuller
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
auto label is removed for flutter/flutter, pr: 117818, due to - Please get at least one approved review if you are already a member or two member reviews if you are not a member before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead. |
|
auto label is removed for flutter/flutter, pr: 117818, due to Validations Fail. |
HansMuller
added
the
autosubmit
6 participants
When I was working on Arna, I saw an opportunity to improve readability of MaterialApp. This PR separates the logic of themebuilder from materialBuilder.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.