-
Notifications
You must be signed in to change notification settings - Fork 8
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
fix(cxl-ui): fix app-layout wide media query #350
Conversation
size-limit report 📦
|
I wonder if we should take the opportunity to try out |
If we were fully lit based, I would say a ReactiveController using ResizeObserver is more effective. But Context would allow us to do it right now. Also, as I mentioned here, I think this requires a proper task to produce an app wide fix. Do you think we could deploy this, as it's very broken in prod now, and work on a Context based solution in the next sprint? |
Yes, but add |
20913cd
to
cf50b1a
Compare
I'm still trying to understand the reasoning behind the Context API. It feels like state management baked into the browser. In this case I would recommend https://github.com/adobe/lit-mobx. Almost zero boilerplate. There wouldn't be much of a performance boost, though we could keep track of media changes across components without having to recalculate, and update accordingly. |
@anoblet You're right about that, enve the Lit team themselves recommend mobX or Redux if we are doing global, shared state management. |
@freudFlintstone If you could create a POC showing the upside of using context, that would be helpful. I'm not sure of the performance gains other than technique. Let me know how I can help 👍 |
We're not looking for performance gains. More DX sanity is the goal. After vaadin-device-detector as the central wide state controller went away, things started to get unglued. We have multiple components interested, and a serious layout bug that snuck live, that's enough signal to me. Seems like worth 1-2h effort simple context system integration should take. |
https://app.clickup.com/t/86ayjqt93