Move MagnifierBuilder, MagnifierOverlayInfoBearer from text_selection.dart#108956
Move MagnifierBuilder, MagnifierOverlayInfoBearer from text_selection.dart#108956auto-submit[bot] merged 14 commits intoflutter:masterfrom
Conversation
|
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. |
|
/cc @antholeole |
|
I can get behind this. I considered making this change multiple times. The reason why I didn't was because I wanted everything in widgets/magnifier.dart to be text-editing agnostic, since it can lead users to think the magnifier can only be used for text editing. That being said, These methods and data classes are tightly coupled to both the magnifier and the selectionOverlay, and could easily go in both places. cc @Renzo-Olivares what are you thoughts? I'm leaning towards approval because this is a change I was going to make myself but opted not to. If it was suggested so quickly, I probably made the wrong choice :) |
Renzo-Olivares
left a comment
There was a problem hiding this comment.
I'm onboard with this change. Cleans up text_selection.dart a bit. Thanks for the change! Just had some small nits.
I think I like these better in |
|
On board! @tgucio do you mind cleaning up those nits? Definitely my mess leftover from my PR, sorry about that. |
|
@antholeole sure thing - will update in a bit. Coud you also maybe add the "automerge" label? Edit: I've thrown in a change in the MagnifierOverlayInfoBearer equality operator and spelling corrections. |
|
test-exempt: code refactor with no semantic change |
|
|
Validations Fail. |
|
Bad bot. |
|
I somehow missed TextMagnifierConfiguration in the original change so moved this one to magnifier.dart too. @antholeole @Renzo-Olivares could you PTAL? |
|
If someone could label this with "autosubmit" again that'd be great. |
This PR moves the MagnifierBuilder, MagnifierOverlayInfoBearer and TextMagnifierConfiguration classes from text_selection.dart to magnifier.dart where they seem to belong better. Additional changes:
test-exempt: refactor only, no functional change.
Pre-launch Checklist
///).