Skip to content

Conversation

@schlawg
Copy link
Contributor

@schlawg schlawg commented Mar 23, 2024

  • use standard css properties in place of #{$start-direction} and #{$end-direction}
  • use mixins that act on html[dir='rtl'] & for everything else
  • gets rid of half of our compiled css
  • cut sass build times in half

After taking this merge, you need to run ui/build --update --clean-theme to get things working again.

The overarching goal is to reduce the size and complexity of an asset manifest needed to support content hashing in filenames. The static themes (light, dark, and transp) will be next to go, and that PR will not be orthogonal. It will encompass and build on this one.

schlawg and others added 9 commits March 23, 2024 16:00
* master:
  Remove chess24 (lichess-org#14962)
  preload the broadcast group name
  tweak hcaptcha skip condition
  better log hcaptcha result type
  rotate oauth token on creation - closes lichess-org#14946
  Add comments for NewTree.toBranch function
  fixes for ios safari < 15
  fix error preventing pure javascript engine when none other are available
this may be completely superfluous,
but I'm concerned not all browsers are capable of optimizing
`:not([dir='rtl'])`
properly, so just in case I added an html class to select instead.

object bits:
val doctype = raw("<!DOCTYPE html>")
def htmlTag(using lang: Lang) = html(st.lang := lang.code, dir := isRTL.option("rtl"))
Copy link
Contributor Author

@schlawg schlawg Mar 25, 2024

Choose a reason for hiding this comment

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

The if-rtl { } mixins are executed at most a few times per page load. Is it really a good idea to optimize away the attribute selector approach? Don't we want exactly one of if-ltr or if-rtl to always match regardless of classes? What about embeds?

Copy link
Collaborator

@ornicar ornicar Apr 2, 2024

Choose a reason for hiding this comment

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

Yeah I'm not sure it's worth it. They do appear quite a lot in the CSS tho:

rg 'dir-(ltr|rtl)' lobby.dark.dev.css | wc -l
14
rg 'dir-(ltr|rtl)' round.dark.dev.css | wc -l
28
rg 'dir-(ltr|rtl)' analyse.relay.dark.dev.css | wc -l
49

And more could be added, as we forget whether or not it's efficient.

ornicar added 2 commits April 2, 2024 13:11
* master: (184 commits)
  upgrade to split scalalib
  fix scalachess version 15.10.0
  Fen.Epd -> Fen.Full
  upgrade scalalib & scalachess
  Remove unneeded path
  remove topnav last link border radius since group has a bottom padding
  define constrain alongside clamp
  Hopefully resolve "ResizeObserver loop limit exceeded" errors seen in user diagnostics
  don't trust the round finished flag
  remove transitive deps
  move unapply to common
  inverte common-core dependency done
  fix dupe streamers in menu, layout flashes
  use consistent boolean for showRatings and fix lichess-org#14997
  Don't wrap token with single-quotes
  invert common->core dependency wip
  refactor common and core
  modules depend on core, not common
  move RawHtml
  move LightUser to core
  ...
* master:
  scalafmt
  Revert "Make Iso independent from common"
  Revert "Move Iso to core"
  Revert "Move Json to core"
  Move Json to core
  Move Iso to core
  Make Iso independent from common
@ornicar ornicar merged commit 4073837 into lichess-org:master Apr 2, 2024
@schlawg schlawg deleted the refactor-css-direction branch April 5, 2024 03:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants