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.

Sign up for GitHub

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

Jump to bottom

Rework !important hack to avoid flicker #600

Merged
merged 1 commit into from
Aug 30, 2024
Merged

Rework !important hack to avoid flicker #600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions frontend/silk-foundation/src/jsMain/kotlin/com/varabyte/kobweb/silk/SilkFoundationStyles.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import androidx.compose.runtime.*
import com.varabyte.kobweb.compose.KobwebComposeStyles
import com.varabyte.kobweb.silk.init.InitSilkContext
import com.varabyte.kobweb.silk.init.initSilk
import com.varabyte.kobweb.silk.style.breakpoint.SilkBreakpointDisplayStyles
import org.jetbrains.compose.web.css.*

/**
Expand Down Expand Up @@ -35,4 +36,5 @@ fun SilkFoundationStyles(initSilk: (InitSilkContext) -> Unit = {}) {
}

Style(SilkStyleSheet)
SilkBreakpointDisplayStyles()
}
73 changes: 0 additions & 73 deletions frontend/silk-foundation/src/jsMain/kotlin/com/varabyte/kobweb/silk/init/InitSilk.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,6 @@ import com.varabyte.kobweb.browser.dom.css.CSSLayerBlockRule
import com.varabyte.kobweb.browser.util.invokeLater
import com.varabyte.kobweb.silk.SilkStyleSheet
import com.varabyte.kobweb.silk.components.text.SpanTextStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayIfAtLeastLgStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayIfAtLeastMdStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayIfAtLeastSmStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayIfAtLeastXlStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayIfAtLeastZeroStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayUntilLgStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayUntilMdStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayUntilSmStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayUntilXlStyle
import com.varabyte.kobweb.silk.style.breakpoint.DisplayUntilZeroStyle
import com.varabyte.kobweb.silk.style.layer.SilkLayer
import com.varabyte.kobweb.silk.theme.ImmutableSilkTheme
import com.varabyte.kobweb.silk.theme.MutableSilkTheme
import com.varabyte.kobweb.silk.theme.SilkTheme
Expand All @@ -23,8 +12,6 @@ import kotlinx.browser.document
import kotlinx.browser.window
import org.w3c.dom.Document
import org.w3c.dom.asList
import org.w3c.dom.css.CSSMediaRule
import org.w3c.dom.css.CSSStyleRule
import org.w3c.dom.css.CSSStyleSheet

/**
Expand Down Expand Up @@ -72,36 +59,6 @@ fun initSilk(additionalInit: (InitSilkContext) -> Unit = {}) {
additionalInit(ctx)
additionalSilkInitialization(ctx)

// Hack alert: Compose HTML does NOT support setting the !important flag on styles, which is in general a good thing
// However, we really want to make an exception for display styles, because if someone uses a method like
// "displayIfAtLeast(MD)" then we want the display to really be none even if inline styles are present.
// Without !important, this code would not work, which isn't expected:
//
// Div(
// Modifier
// .displayIfAtLeast(MD)
// .grid { row(1.fr); column(1.fr) }
// )
//
// `grid` sets the display type to "grid", which overrides the `display: none` from `displayIfAtLeast`
// See below for where we find these styles and update them to use !important.
val displayStyles = listOf(
DisplayIfAtLeastZeroStyle to "silk-display-if-at-least-zero",
DisplayIfAtLeastSmStyle to "silk-display-if-at-least-sm",
DisplayIfAtLeastMdStyle to "silk-display-if-at-least-md",
DisplayIfAtLeastLgStyle to "silk-display-if-at-least-lg",
DisplayIfAtLeastXlStyle to "silk-display-if-at-least-xl",
DisplayUntilZeroStyle to "silk-display-until-zero",
DisplayUntilSmStyle to "silk-display-until-sm",
DisplayUntilMdStyle to "silk-display-until-md",
DisplayUntilLgStyle to "silk-display-until-lg",
DisplayUntilXlStyle to "silk-display-until-xl",
)

displayStyles.forEach { (style, name) ->
mutableTheme.registerStyle(name, style)
}

MutableSilkConfigInstance = config

_SilkTheme = ImmutableSilkTheme(mutableTheme)
Expand Down Expand Up @@ -141,36 +98,6 @@ fun initSilk(additionalInit: (InitSilkContext) -> Unit = {}) {
}
}

run {
// Run through all styles in the stylesheet and update the ones associated with our display styles, making
// them important. This means that responsive designs will always work -- that another style that sets the
// `display` property will never accidentally overrule it.
// Note that a real solution would be if the Compose HTML APIs allowed us to identify a style as important,
// but currently, as you can see with their code here:
// https://github.com/JetBrains/compose-multiplatform/blob/9e25001e9e3a6be96668e38c7f0bd222c54d1388/html/core/src/jsMain/kotlin/org/jetbrains/compose/web/elements/Style.kt#L116
// they don't support it. (It would have been nice to be a version of the API that takes an additional
// priority parameter, as in `setProperty("x", "y", "important")`)
val displayStyleSelectorNames = displayStyles.map { (_, name) -> ".${name}" }.toSet()
document.localStyleSheets
.flatMap { styleSheet ->
// Note: We know all display styles use media rules & layers blocks, but if we ever want to support
// "important" more generally, we'd have to handle at least rules at all levels.
styleSheet.cssRules.asList()
.filterIsInstance<CSSMediaRule>()
.flatMap { it.cssRules.asList() }
.mapNotNull { rule ->
(rule as? CSSLayerBlockRule)?.takeIf { it.name == SilkLayer.GENERAL_STYLES.layerName }
}.flatMap { it.cssRules.asList().filterIsInstance<CSSStyleRule>() }
}.forEach { rule ->
val selectorText = rule.selectorText
val innerStyle = rule.style
if (selectorText in displayStyleSelectorNames) {
val displayValue = innerStyle.getPropertyValue("display")
innerStyle.setProperty("display", displayValue, "important")
}
}
}

document.styleSheets.asList()
.filterIsInstance<CSSStyleSheet>()
// Trying to peek at external stylesheets causes a security exception so step over them
Expand Down
91 changes: 53 additions & 38 deletions ...ation/src/jsMain/kotlin/com/varabyte/kobweb/silk/style/breakpoint/BreakpointConditions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.varabyte.kobweb.silk.style.breakpoint

import androidx.compose.runtime.*
import com.varabyte.kobweb.compose.dom.GenericTag
import com.varabyte.kobweb.compose.ui.Modifier
import com.varabyte.kobweb.compose.ui.modifiers.*
import com.varabyte.kobweb.silk.style.CssStyle
import com.varabyte.kobweb.silk.theme.breakpoint.toMinWidthQuery
import org.jetbrains.compose.web.css.*

import org.w3c.dom.HTMLStyleElement
import org.w3c.dom.css.CSSRule
import org.w3c.dom.css.CSSStyleSheet

private fun CSSMediaQuery.invert(): CSSMediaQuery {
// Note: We invert a min-width query instead of using a max-width query because otherwise there's a width where
Expand All @@ -15,46 +18,58 @@ private fun CSSMediaQuery.invert(): CSSMediaQuery {
return CSSMediaQuery.Raw("not all and $this")
}

// Technically unnecessary -- this just means always display. But provided for completion for all breakpoint values.
internal val DisplayIfAtLeastZeroStyle = CssStyle {
cssRule(Breakpoint.ZERO.toMinWidthQuery().invert()) { Modifier.display(DisplayStyle.None) }
}

internal val DisplayIfAtLeastSmStyle = CssStyle {
cssRule(Breakpoint.SM.toMinWidthQuery().invert()) { Modifier.display(DisplayStyle.None) }
}

internal val DisplayIfAtLeastMdStyle = CssStyle {
cssRule(Breakpoint.MD.toMinWidthQuery().invert()) { Modifier.display(DisplayStyle.None) }
}

internal val DisplayIfAtLeastLgStyle = CssStyle {
cssRule(Breakpoint.LG.toMinWidthQuery().invert()) { Modifier.display(DisplayStyle.None) }
}

internal val DisplayIfAtLeastXlStyle = CssStyle {
cssRule(Breakpoint.XL.toMinWidthQuery().invert()) { Modifier.display(DisplayStyle.None) }
}

// Technically unnecessary -- this just means never display. But provided for completion for all breakpoint values.
internal val DisplayUntilZeroStyle = CssStyle {
Breakpoint.ZERO { Modifier.display(DisplayStyle.None) }
}

internal val DisplayUntilSmStyle = CssStyle {
Breakpoint.SM { Modifier.display(DisplayStyle.None) }
}

internal val DisplayUntilMdStyle = CssStyle {
Breakpoint.MD { Modifier.display(DisplayStyle.None) }
// Hack alert: Compose HTML does NOT support setting the !important flag on styles, which is in general a good thing
// However, we really want to make an exception for display styles, because if someone uses a method like
// "displayIfAtLeast(MD)" then we want the display to really be none even if inline styles are present.
// Without !important, this code would not work, which isn't expected:
//
// Div(
// Modifier
// .displayIfAtLeast(MD)
// .grid { row(1.fr); column(1.fr) }
// )
//
// `grid` sets the display type to "grid", which overrides the `display: none` from `displayIfAtLeast`
//
// Note that a real solution would be if the Compose HTML APIs allowed us to identify a style as important,
// but currently, as you can see with their code here:
// https://github.com/JetBrains/compose-multiplatform/blob/9e25001e9e3a6be96668e38c7f0bd222c54d1388/html/core/src/jsMain/kotlin/org/jetbrains/compose/web/elements/Style.kt#L116
// they don't support it. (It would have been nice to be a version of the API that takes an additional
// priority parameter, as in `setProperty("x", "y", "important")`)
//
// Below, we recreate Compose HTML's `Style()` composable, but directly set the styles we want to mark `!important`.
// https://github.com/JetBrains/compose-multiplatform/blob/9e25001e9e3a6be96668e38c7f0bd222c54d1388/html/core/src/jsMain/kotlin/org/jetbrains/compose/web/elements/Elements.kt#L979
@Composable
internal fun SilkBreakpointDisplayStyles() {
GenericTag("style") {
DisposableEffect(Unit) {
val cssStylesheet = scopeElement.unsafeCast<HTMLStyleElement>().sheet as? CSSStyleSheet
Breakpoint.entries.forEach { breakpoint ->
// `display-if-at-least-zero` and `display-until-zero` are unnecessary, but provided for completeness
val minWidthQuery = CSSMediaRuleDeclaration(breakpoint.toMinWidthQuery(), emptyList()).header
val invertMinWidthQuery =
CSSMediaRuleDeclaration(breakpoint.toMinWidthQuery().invert(), emptyList()).header
cssStylesheet
?.addRule("$invertMinWidthQuery { .silk-display-if-at-least-${breakpoint.name.lowercase()} { display: none !important; } }")
cssStylesheet
?.addRule("$minWidthQuery { .silk-display-until-${breakpoint.name.lowercase()} { display: none !important; } }")
}
onDispose {
cssStylesheet?.clearCSSRules()
}
}
}
}

internal val DisplayUntilLgStyle = CssStyle {
Breakpoint.LG { Modifier.display(DisplayStyle.None) }
private fun CSSStyleSheet.addRule(cssRule: String): CSSRule? {
val cssRuleIndex = this.insertRule(cssRule, this.cssRules.length)
return this.cssRules.item(cssRuleIndex)
}

internal val DisplayUntilXlStyle = CssStyle {
Breakpoint.XL { Modifier.display(DisplayStyle.None) }
private fun CSSStyleSheet.clearCSSRules() {
repeat(cssRules.length) {
deleteRule(0)
}
}

fun Modifier.displayIfAtLeast(breakpoint: Breakpoint) =
Expand Down