-
-
Notifications
You must be signed in to change notification settings - Fork 354
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 MUI theme in Jupyter AI Settings #1210
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MUFFANUJ Thank you for contributing this, and welcome to the Jupyter AI project! 🤗 Reading from the CSS variables provided by JupyterLab to build a custom theme is exactly what we did in Jupyter AI v2. Your approach is correct, and I appreciate the time you took to write a description & provide a demo video as a new contributor. This is a great first contribution.
I did not mention that we still have the v2 implementation for setting the Material UI theme based on JupyterLab's theme. This JlThemeProvider
component provides the styles through a React context, so all child elements are styled automatically without the need to add custom CSS to every element. The JlThemeProvider
accidentally became unused during the initial v2 => v3 migration. Therefore, it would be better to re-introduce JlThemeProvider
instead of adding custom CSS.
I've opened a new PR which performs these code changes: #1213. This provides a nicer UI with fewer lines of code by leveraging our existing work.
Since this is your first contribution: Feel free to copy my changes over into your PR, and I'll merge yours in favor of mine. It doesn't feel right to close the very first PR you opened for us, and I think you deserve credit for being the first to work on this issue.
- If you'd rather start working on something else, check out this milestone of good first issues. It only has 2 right now, but I will ask my team to help find more to add: https://github.com/jupyterlab/jupyter-ai/milestone/13
Thank you so much for your kind words and suggestions @dlqqq @srdas ! 😊 I truly appreciate the time and effort you’ve taken to review my contribution, provide such detailed feedback, and even create an alternative PR. I sincerely apologize for not getting complete clarity about the issue beforehand—it’s a lesson for me to always double-check and ensure alignment before starting. I’ve started understanding the codebase better little by little, and this experience has been incredibly helpful for my learning process. I’ll go through the changes you’ve made in #1213 and study the approach with the JlThemeProvider to understand it more deeply. I’ll also keep your suggestion in mind for future contributions and will actively seek guidance from the community to make sure I’m aligned with the project goals. Once again, thank you for your support and for making my first contribution such a positive experience. |
9022074
to
43d3175
Compare
8cba387
to
da44492
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work, thank you! 🎉
No need to apologize about missing something; that's what code review is for. 🤗 Your PRs will get better if you read more code & understand what is happening.
fixes - #1175
This PR addresses the issue where the Jupyter AI settings panel does not properly utilize the JupyterLab theme provider, resulting in incorrect styling of components.
Changes:
Integrated the JupyterLab theme provider to ensure components follow the JupyterLab theme styles.
Corrected rendering issues in the dark theme to ensure proper visibility and contrast of components.
This update ensures that the settings panel is visually consistent with the rest of the JupyterLab interface in both light and dark themes.
Here is the video demo of the changes made:-
https://github.com/user-attachments/assets/135a317c-160f-4a05-b76d-61fe1d281717