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

[web] Remove unnecessary CSS styles on <p> elements #32043

Merged
merged 9 commits into from
Mar 28, 2022

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Mar 15, 2022

With #31907, we started to absolutely position all the spans inside the paragraph, so there's no layout/alignment happening by the DOM. This means we can remove all the CSS styles that were used to make the browser lay things out correctly.

  • Rename <p> and <span> elements to <flt-paragraph> and <flt-span> respectively.
  • Remove a whole bunch of CSS that we don't need anymore.
  • Fix tests.

@flutter-dashboard flutter-dashboard bot added the platform-web Code specifically for the web engine label Mar 15, 2022
@mdebbar mdebbar changed the title Style cleanup [web] Remove unnecessary CSS styles on <p> elements Mar 15, 2022
@skia-gold
Copy link

Gold has detected about 5 new digest(s) on patchset 6.
View them at https://flutter-engine-gold.skia.org/cl/github/32043

@mdebbar mdebbar requested review from yjbanov and ditman and removed request for yjbanov March 16, 2022 17:13
@mdebbar mdebbar marked this pull request as ready for review March 16, 2022 17:13
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

LGTM! Small question about the "new spans"

Comment on lines -178 to +176
lastSpanElement = html.document.createElement('span') as html.HtmlElement;
lastSpanElement = html.document.createElement('flt-span') as html.HtmlElement;
Copy link
Member

Choose a reason for hiding this comment

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

Custom elements behave like divs right? Do you need display: inline (or inline-block) to retain what was special of using a span tag earlier? Or are we just doing display:block on everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point.

The display property doesn't matter anymore. Each <flt-span> contains a single fragment of text that can't be broken into multiple lines.

A single line of text that is positioned absolutely shouldn't be affected by display:block or display:inline.

@skia-gold
Copy link

Gold has detected about 6 new digest(s) on patchset 9.
View them at https://flutter-engine-gold.skia.org/cl/github/32043

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
platform-web Code specifically for the web engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants