Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use proper css for language direction #14951

Merged
merged 11 commits into from Apr 2, 2024

Conversation

schlawg
Copy link
Collaborator

@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.
@@ -15,10 +15,12 @@ import lila.common.base.StringUtils.escapeHtmlRaw
object layout:

object bits:
val doctype = raw("<!DOCTYPE html>")
def htmlTag(using lang: Lang) = html(st.lang := lang.code, dir := isRTL.option("rtl"))
Copy link
Collaborator 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.

* 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
5 checks passed
@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.

None yet

2 participants