atellmer / dark

The lightweight and powerful UI rendering engine without dependencies and written in TypeScriptđź’« (Browser, Node.js, Android, iOS, Windows, Linux, macOS)
MIT License
40 stars 1 forks source link

Media queries and styled #63

Closed Coachonko closed 1 month ago

Coachonko commented 2 months ago

The following style

import { Link } from '@dark-engine/web-router'
import { styled, css } from '@dark-engine/styled'

const disabledStyle = ({ disabled }) => {
  if (disabled) {
    return css`
    pointer-events: none;
    opacity: .2;`
  }
  return css`
  pointer-events: auto;
  opacity: 1;`
}

const underlineStyle = ({ underline }) => {
  if (underline) {
    return css`text-decoration: none;
    background-image: linear-gradient(currentColor,currentColor);
    background-repeat: no-repeat;
    transition: background-size .3s;`
  }
}

const activeStyle = ({ active }) => {
  if (active) {
    return css`
    background-position: 0 100%;
    background-size: 100% 1px;
    @media(hover: hover) {
      &:hover {
        background-position:100% 100%;
        background-size: 0 1px
      }
    }

    @media(hover: none) {
      &:active {
        background-position:100% 100%;
        background-size: 0 1px
      }
    }`
  }

  return css`
  background-position: 100% 100%;
  background-size: 0 1px;
  @media(hover: hover) {
    &:hover {
        background-position:0 100%;
        background-size: 100% 1px;
    }
  }

  @media(hover: none) {
    &:active {
        background-position:0 100%;
        background-size: 100% 1px;
    }
  }`
}

export const button = ({ disabled, underline, active }) => css`
  position: relative;
  display: inline-flex;
  align-items: center;

  ${disabledStyle({ disabled })}
  ${underlineStyle({ underline })}
  ${activeStyle({ active })}`

const StyledLink = styled(Link)`
  ${({ disabled, active }) => button({ disabled, underline: true, active })}
`

Results into this in the browser

.dk-jeihhc {
  position: relative;
  display: inline-flex;
  align-items: center;
  pointer-events: auto;
  opacity: 1;
  text-decoration: none;
  background-image: linear-gradient(currentColor, currentColor);
  background-repeat: no-repeat;
  transition: background-size .3s;
  background-position: 100% 100%;
  background-size: 0 1px;
}

@media(hover: hover) {
  .dk-jeihhc {}

  .null:hover {
    background-position: 0 100%;
    background-size: 100% 1px;
  }
}

@media(hover: none) {
  .dk-jeihhc {}

  .null:active {
    background-position: 0 100%;
    background-size: 100% 1px;
  }
}

.dk-jeihhc {
  position: relative;
  display: inline-flex;
  align-items: center;
  pointer-events: auto;
  opacity: 1;
  text-decoration: none;
  background-image: linear-gradient(currentColor, currentColor);
  background-repeat: no-repeat;
  transition: background-size .3s;
  background-position: 100% 100%;
  background-size: 0 1px;
}

@media(hover: hover) {
  .dk-jeihhc {}

  .null:hover {
    background-position: 0 100%;
    background-size: 100% 1px;
  }
}

@media(hover: none) {
  .dk-jeihhc {}

  .null:active {
    background-position: 0 100%;
    background-size: 100% 1px;
  }
}

I see a few weird things happening, but most importantly, the media queries are broken. &:hover becomes .null:hover Am I doing it wrong?

atellmer commented 2 months ago

You are inserting a call to one css function into another, this is not supported. At least it never occurred to me why this would be necessary. Your approach is very different from styled-components. You wrap everything in one dynamic function, which means that the styles will be duplicated in the style tag. Try to separate static styles from dynamic ones. Try it:

const disabledStyle = ({ disabled }) => {
  if (disabled) {
    return css`
      pointer-events: none;
      opacity: 0.2;
    `;
  }

  return css`
    pointer-events: auto;
    opacity: 1;
  `;
};

const underlineStyle = ({ underline }) => {
  if (underline) {
    return css`
      text-decoration: none;
      background-image: linear-gradient(currentColor, currentColor);
      background-repeat: no-repeat;
      transition: background-size 0.3s;
    `;
  }

  return '';
};

const activeStyle = ({ active }) => {
  if (active) {
    return css`
      background-position: 0 100%;
      background-size: 100% 1px;

      @media (hover: hover) {
        &:hover {
          background-position: 100% 100%;
          background-size: 0 1px;
        }
      }

      @media (hover: none) {
        &:active {
          background-position: 100% 100%;
          background-size: 0 1px;
        }
      }
    `;
  }

  return css`
    background-position: 100% 100%;
    background-size: 0 1px;

    @media (hover: hover) {
      &:hover {
        background-position: 0 100%;
        background-size: 100% 1px;
      }
    }

    @media (hover: none) {
      &:active {
        background-position: 0 100%;
        background-size: 100% 1px;
      }
    }
  `;
};

const buttonStyle = () => css`
  position: relative;
  display: inline-flex;
  align-items: center;
`;

const StyledLink = styled(Link)`
  // first (static) class .dk-behaba
  ${buttonStyle()}
  ${underlineStyle({ underline: true })}
  // second (dynamic) class .dk-badcfa
  ${({ disabled }) => disabledStyle({ disabled })}
  // third (dynamic) class .dk-hdbbge
  ${({ active }) => activeStyle({ active })}
`;
<style>
    .dk-behaba {
      position: relative;
      display: inline-flex;
      align-items: center;
      text-decoration: none;
      background-image: linear-gradient(currentColor, currentColor);
      background-repeat: no-repeat;
      transition: background-size 0.3s;
    }

    .dk-badcfa {
      pointer-events: auto;
      opacity: 1;
    }

    .dk-hdbbge {
      background-position: 100% 100%;
      background-size: 0 1px;
    }

    @media (hover: hover) {
      .dk-hdbbge {}

      .dk-hdbbge:hover {
        background-position: 0 100%;
        background-size: 100% 1px;
      }
    }

    @media (hover: none) {
      .dk-hdbbge {}

      .dk-hdbbge:active {
        background-position: 0 100%;
        background-size: 100% 1px;
      }
    }

    .dk-bbgjch {
      background-position: 0 100%;
      background-size: 100% 1px;
    }

    @media (hover: hover) {
      .dk-bbgjch {}

      .dk-bbgjch:hover {
        background-position: 100% 100%;
        background-size: 0 1px;
      }
    }

    @media (hover: none) {
      .dk-bbgjch {}

      .dk-bbgjch:active {
        background-position: 100% 100%;
        background-size: 0 1px;
      }
    }

    .dk-bjfhbc {
      pointer-events: none;
      opacity: 0.2;
    }
  </style>
atellmer commented 2 months ago

Another approach

const buttonStyle = () => css`
  position: relative;
  display: inline-flex;
  align-items: center;
`;

const Button = styled.button`
  ${buttonStyle()}
  ${underlineStyle({ underline: true })}
  ${({ disabled }) => disabledStyle({ disabled })}
  ${({ active }) => activeStyle({ active })}
`;

const mix = (base, target) => component(props => base({ as: target, ...props }));

const StyledLink = mix(Button, Link); // is Link with Button styles (but it creates one more component)
Coachonko commented 2 months ago

Got it thank you :+1: Maybe an error should be thrown then, because I couldn't understand what was wrong

Can you please explain what causes styles to be duplicated? I have fixed this portion as suggested but my styles are still being duplicated, I can't find the source of the duplication.

I think I understand that the styles inside my page transition component, discussed in another issue, are all duplicated. I am not sure, I am just speculating, that the cause is animations/useTransition.

atellmer commented 2 months ago

Can you give an example of duplicates in the style tag in the browser? And the code itself that generates them.

As far as I remember, any dynamic style (based on props) is a separate class and if you change props often, then it can generate the same styles in the DOM. Therefore, it is highly not recommended to use a dynamic style with frequently changing unique values, for example margin-top: 5px -> 6px -> 7px -> ... -> 20px. In this case, a new class with all internal styles will be generated each time. To do this you need to use the style attribute.

Coachonko commented 2 months ago

I am trying to isolate and replicate. So far I know these things are happening in my app:

  1. Even empty styled components, used only once, are being duplicated. As long as they appear inside the animated component.
  2. The server render is not affected.
  3. GlobalStyles are not affected.
const NonDuplicatedCheck = styled.div`margin: 50px;`

const Root = component(({ slot }) => {
  return (
    <>
      <GlobalStyle /> {/* non duplicated */}
      <ChangeTitle />
      <ScrollToTop />

      <NonDuplicatedCheck /> {/* non duplicated */}
      <PageTransition> {/* any style in here is duplicated */}
        <AppLayout>
          {slot}
        </AppLayout>
      </PageTransition>
    </>
  )
})

const StyledHeader = styled.header`margin: 20px;`
const StyledMain = styled.main``

const AppLayout = component(({ slot }) => {
  return (
    <>
      <StyledHeader> {/* duplicated */}
        <HeaderNav />
      </StyledHeader>
      <StyledMain> {/* duplicated */}
        {slot}
      </StyledMain>
      <footer>
        <FooterTop />
        <FooterNav />
        <FooterBottom />
      </footer>
    </>
  )
})

Result from server (does not include GlobalStyles)

.dk-bjgbig {
  margin: 50px;
}

.dk-bgbbcg {
  margin: 20px;
}

.dk-cagiea {}

Result on browser (does not include GlobalStyles)

.dk-bjgbig {
  margin: 50px;
}

.dk-bgbbcg {
  margin: 20px;
}

.dk-cagiea {}

.dk-bgbbcg {
  margin: 20px;
}

.dk-cagiea {}

So I am thinking it has something to do with the PageTransition component, same component from here

atellmer commented 2 months ago

Ok. Thanks. I will check it and try to fix.

atellmer commented 1 month ago

I tried to reproduce the problem and was unable to do so. I think that I can’t do without a working example so that I can simply clone the repository and see your problem.

atellmer commented 1 month ago

In the new version css function call will be available within another css functions call

const size = (s = 100) => css`
  width: ${s}px;
  height: ${s}px;
`;

const hover = () => css`
  transition: background-color 0.2s ease-in-out;

  &:hover {
    background-color: red;
  }
`;

const color = (c = '#fff') => css`
  background-color: ${c};
  ${hover()} // css call within a css call
`;

const Box = styled.div<{ $size: number; $color: string } & DarkJSX.Elements['div']>`
  ${({ $size }) => size($size)}
  ${({ $color }) => color($color)}
`;

const App = component(() => {
  return (
    <>
      <Box $size={100} $color='green' />
      <Box $size={100} $color='yellow' />
    </>
  );
});
Coachonko commented 1 month ago

Hi sorry I was away last week.

I tried to reproduce the problem and was unable to do so. I think that I can’t do without a working example so that I can simply clone the repository and see your problem.

I will create a "minimal" repro repository for you

Coachonko commented 1 month ago

I also noticed that

const baseCSS = () => css`
  @media(hover: hover) {
    &:hover {
      background-position: 0 100%;
      background-size: 100% 1px;
    }
  }
`

results in

@media(hover: hover) {
  .dk-jaihfc {} /* weird empty rule  */

  .dk-jaihfc:hover {
    background-position: 0 100%;
    background-size: 100% 1px;
  }
}
atellmer commented 1 month ago

Empty rules are normal when a NestingRule (media for instance) does not contain its own style rules but only another nestings

Coachonko commented 1 month ago

I have pushed an update to my boilerplate repository, this update exhibits the duplicated styles problem described above. I have not created a more minimal repro yet

Coachonko commented 1 month ago

I have begun trying to "skim down" the boilerplate, to make it more minimal. here I have reproducing 2 bugs I have run into, one of which is the duplicated styles one

atellmer commented 1 month ago

Thanks, I have already fixed the duplicates. The reason was not PageTransitions, but hydration. I'm closing this issue. For new errors - new issue, so as not to get confused.

Coachonko commented 1 month ago

Thanks, I have already fixed the duplicates. The reason was not PageTransitions, but hydration.

Oh cool, thanks. That's interesting, it only happened in components inside my PageTransition component so I assumed that was the issue, thank you for fixing it, that will make working in devtools easier

atellmer commented 1 month ago

v1.3.0 is available

Coachonko commented 1 month ago

I can confirm all duplicated styles are gone. Awesome!

I've been adding more and more styles to a project and ran into something I don't understand

// Note: $scrollbarWidth is === 0 on the server, === 15 on my browser
const StyledHeader = styled.header`
  position: absolute;
  transition: transform .3s, background-color .3s;
  transform: translateY(-100%);
  background-color: ${colors.white};
  z-index: ${zIndex.header};
  padding: 1vw 0;
  ${/*problem begin*/}
  ${props => {
    // return css`inset: 0 0 auto;` // this does work
    // return css`inset: 0 ${props.$scrollbarWidth}px auto 0;` // this works on the server, but not on the browser
    if (props.$scrollbarWidth === 0) {
      console.log(css`inset: 0 0 auto;`) // outputs correctly
      return css`inset: 0 0 auto;` // appears in <style> but StyledHeader does not get this class in the browser, residual from SSR
    }
    console.log(css`inset: 0 ${props.$scrollbarWidth}px auto 0;`) // outputs correctly
    return css`inset: 0 ${props.$scrollbarWidth}px auto 0;` // not rendered, missing from <style> too
  }};
  ${/*problem end*/}
  ${props => { // this works
    if (props.$isAtTop === true) {
      return css`background-color: ${colors.white}00;`
    }
  }};
  ${props => { // this works
    if (props.$isVisible === true) {
      return css`transform: translateY(0);`
    }
  }};
@media(prefers-reduced-motion: reduce) {
  transform: translateY(0);
  background-color: ${colors.white};
}

Please do not mind the verbose syntax, I can drop-in console.log easily right now

I do not think I am doing something wrong here, is this a bug? I am experiencing the same issue on version 1.2.0, so this is probably not related to 1.3.0 changes

atellmer commented 1 month ago

Looks like a bug. I did not take into account that the server version of the styles does not always match the browser, although ideally it should. In this case, I need to further check the styles during the hydration process.

Coachonko commented 1 month ago

After more testing, I find it even more weird

This is what I observed yesterday

  // in component
  const scrollbarWidth = useScrollbarWidth()
  console.log(scrollbarWidth, typeof scrollbarWidth) // on server: 0 number, on browser 15 number
  return <StyledHeader $isVisible={isVisible} $scrollbarWidth={scrollbarWidth} />

  // style
  ${props => {
    console.log(props.$scrollbarWidth, typeof props.$scrollbarWidth) // on server: 0 number, on browser 15 number
    if (props.$scrollbarWidth === 0) {
      return css`inset: 0 0 auto;` // server includes this, as expected. Browser doesn't use this.
    }
    return css`inset: 0 ${props.$scrollbarWidth}px auto 0;` // browser does not generate this
  }};

Today I tried this:

  // in component
  return <StyledHeader $isVisible={isVisible} $scrollbarWidth={15} /> // hard 15

  // style
  ${props => {
    console.log(props.$scrollbarWidth, typeof props.$scrollbarWidth) // on server: 0 number, on browser 15 number (no change)
    if (props.$scrollbarWidth === 0) {
      return css`inset: 0 0 auto;` // server includes this, browser removes it
    }
    return css`inset: 0 ${props.$scrollbarWidth}px auto 0;` // browser generates and uses this
  }};

In both cases, the variable is a number, but only in the second case things work as expected :confused:

atellmer commented 1 month ago

return <StyledHeader $isVisible={isVisible} $scrollbarWidth={15} /> // hard 15

I bet the server hasn't been rebuilt here. There cannot be 0 if 15 is hardcoded.

Anyway, I fixed it.

Coachonko commented 1 month ago

return <StyledHeader $isVisible={isVisible} $scrollbarWidth={15} /> // hard 15

I bet the server hasn't been rebuilt here. There cannot be 0 if 15 is hardcoded.

Anyway, I fixed it.

You're right on that. I apologize for the oversight. This bug made me crazy

Coachonko commented 1 month ago

I was was testing media queries again using theme variables (typically one would likely use :root custom properties), do you see anything wrong with the media query? It doesn't render at all

const Hero = styled.section`
  height: calc(100svh - ${props => props.theme.headerHeight});

  @media (min-width: ${props => props.theme.sm}px) {
    height: calc(100vh - ${props => props.theme.headerHeightSm});
  }
`
atellmer commented 1 month ago

Try

const Hero = styled.section`
  height: ${props => `calc(100svh - ${props.theme.headerHeight})`}; // prop value

  ${props => css`
    // css fragment
    @media (min-width: ${props.theme.sm}px) {
      height: calc(100vh - ${props.theme.headerHeightSm});
    }
  `}
`;

Functions cannot be inserted anywhere, because they are parsed and not simply executed in js. The function can be inserted only as a property value, or as a complete css fragment.

Coachonko commented 1 month ago

I see. background-color: ${props => props.theme.white}00; this works though, so I assumed they could be inserted anywhere