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

Palette page lags when selecting bg color #44

Open
CarlosBalladares opened this issue Jun 26, 2022 · 10 comments
Open

Palette page lags when selecting bg color #44

CarlosBalladares opened this issue Jun 26, 2022 · 10 comments
Labels
bug Something isn't working

Comments

@CarlosBalladares
Copy link

Describe the bug
Whenever you try to select a bg color the palette page lags,

To Reproduce
Steps to reproduce the behavior:

  1. Create a palette
  2. go to palette page
  3. select background color with input with id 'bg-color', click on the color selection prompt and drag the cursor, see the lag as the values change

Expected behavior
Frames should not drop and the ui should not look laggy/choppy

Environment
Windows 10 chrome 102

Additional context
Reproducible if you run on localhost as well

@aashutoshrathi
Copy link

aashutoshrathi commented Jun 26, 2022

@pouretrebelle The reason for the lag is every component(button, icons, text, etc) changes its hover/bg/text color based on the selection in the color picker for the background.
One way to reduce this lag is to debounce the update of color for 500ms

I'll be happy to try to fix it if you are already not working on it.

Views? @colebemis

@colebemis
Copy link
Contributor

Debouncing the update seems like a great idea @aashutoshrathi! Would be a happy to review a PR for this

@colebemis colebemis added the bug Something isn't working label Jun 27, 2022
@aashutoshrathi
Copy link

Update on this seems like it is already being debounced (200ms by default in GlobalState setters).
The issue is different, I think it is because the style of wrapper div changes on every update, in turn creating a new class.
After some time, it basically has more than 200 classes for it. I can try to give it a static className.

Adding the piece of code, I'm talking about.

const Wrapper = styled.div<{backgroundColor: string}>`
--color-text: ${props => readableColor(props.backgroundColor)};
--color-background: ${props => props.backgroundColor};
--color-background-secondary: ${props => mix(readableColor(props.backgroundColor), props.backgroundColor, 0.9)};
--color-background-secondary-hover: ${props =>
mix(readableColor(props.backgroundColor), props.backgroundColor, 0.85)};
--color-border: ${props => mix(readableColor(props.backgroundColor), props.backgroundColor, 0.75)};
display: grid;
grid-template-columns: 300px 1fr;
grid-template-rows: auto 1fr;
grid-template-areas: 'header header' 'sidebar main';
color: var(--color-text);
background-color: var(--color-background);
height: 100vh;
`

@colebemis
Copy link
Contributor

Ah, @aashutoshrathi what if we set those variables using inline styles on the Wrapper instead of inside the styled component definition? Like this:

<Wrapper style={{ '--color-text': readableColor(props.backgroundColor), ...}}>

I think this would prevent unnecessary regeneration of classNames. @aashutoshrathi can you check if this improves the performance?

@aashutoshrathi
Copy link

aashutoshrathi commented Jun 27, 2022

Confirming that the performance improves when I declare CSS vars inline. But the catch is, it slows down when we open one of Scales
cc: @colebemis

@CarlosBalladares
Copy link
Author

was the website taken down?

@colebemis
Copy link
Contributor

colebemis commented Jun 28, 2022

Looking into the deploy issue now @CarlosBalladares Should be working now 👍

@colebemis
Copy link
Contributor

But the catch it, it slows down when we open one of Scales

Hmm, any idea why?

@aashutoshrathi
Copy link

Inspecting the Scales component, since that's the part causing lag. I will push a patch, if as soon as I find a reason

@aashutoshrathi
Copy link

@colebemis, I tried finding the cause of lag in the Scale component, seems like its too big and has many points of failure, but temporarily I would suggest that we atleast make bgColor change faster for the case when the scale is not opened.
You can check my branch for that fix over here: #50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants