Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@joshualitt
Copy link
Contributor

This is CL 1 in a series of CLs to migrate Flutter Web DOM usage to the new JS static interop API.

Note: Because we have not quite finished the new DOM library based on JS static interop, this CL introduces a shim library which should mostly match the forthcoming API. A subsequent much smaller migration will occur once the new library is complete.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Apr 14, 2022
@joshualitt joshualitt force-pushed the html_1 branch 5 times, most recently from b7700a4 to ee86be3 Compare April 15, 2022 01:10
@joshualitt
Copy link
Contributor Author

joshualitt commented Apr 15, 2022

@srujzs @rileyporter ptal. Unfortunately, due to how the engine is built, I can't use an import prefix, so I had to name space dom.dart the old fashioned way.

Please pay particular attention to nullability.

Copy link
Contributor

@rileyporter rileyporter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, it's interesting to see what you've needed in dom.dart. I'll be curious to see what else you end up needing.

@joshualitt joshualitt marked this pull request as ready for review April 20, 2022 23:25
@flutter-dashboard
Copy link

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.

@joshualitt
Copy link
Contributor Author

@yjbanov @eyebrowsoffire ptal. I think it would be a good idea for both of you to look at this first cl. The rest of the CLs look a lot like this one.

@Hixie
Copy link
Contributor

Hixie commented Apr 25, 2022

test-exempt: code refactor with no semantic change

@joshualitt joshualitt removed the request for review from yjbanov April 27, 2022 20:40
@joshualitt
Copy link
Contributor Author

@yjbanov , I'm going to land this just to see if there are any hiccups with this approach. If you have any feedback on this CL, please let me know and I will be happy to address it in one of the numerous follow ons.

@joshualitt joshualitt merged commit dc2a7da into flutter:main Apr 28, 2022
@joshualitt joshualitt deleted the html_1 branch April 28, 2022 15:04
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs tests platform-web Code specifically for the web engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants