Skip to content

Conversation

hemanth5055
Copy link
Contributor

@hemanth5055 hemanth5055 commented Aug 26, 2025

Description

This PR addresses issue #13.

What I did

  • Added a new prism-light.css file for proper light-mode syntax highlighting.
  • Updated content.js to read the theme from chrome.storage.sync.
  • Applied a theme class (.light / .dark) to the injected UI’s Shadow DOM container.
  • Updated ui.js to load the appropriate Prism stylesheet based on the theme.

Current status
The code compiles and injects, but the injected UI still doesn’t consistently reflect the saved theme. Sometimes it falls back to default styling.

Request for review
I’d appreciate a review to confirm if I’m:

  • Accessing chrome.storage.sync correctly from the content script.
  • Injecting the CSS at the right stage of the Shadow DOM lifecycle.
  • Following the right approach for handling multiple Prism CSS files (instead of duplicating styles).

Sometimes it works :

Screenshot 2025-08-26 at 9 15 19 PM Screenshot 2025-08-26 at 9 15 41 PM

@dineshsutihar dineshsutihar self-requested a review August 27, 2025 18:51
Copy link
Collaborator

@dineshsutihar dineshsutihar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change line 61 in content.js to:
injectOrUpdateTranslations(newData, clickedElement, originalWidth, theme);
then it should work

@dineshsutihar
Copy link
Collaborator

Hi! Thank you so much for this fantastic contribution and for the incredibly detailed description.
You've done a great job here, and your approach is spot on. You've found the exact cause of the inconsistent behavior, and it's due to one tiny missing variable.

In content.js, inside the chrome.runtime.sendMessage callback, you just need to pass the theme variable to the final injectOrUpdateTranslations call.

The fix:
Change this line:
injectOrUpdateTranslations(newData, clickedElement, originalWidth);
To this:
injectOrUpdateTranslations(newData, clickedElement, originalWidth, theme);

That's it! That one change will make the theming work consistently.
To answer your questions directly:

  • Accessing chrome.storage.sync: Your approach is perfect.
  • Injecting CSS in the Shadow DOM: You are doing this at the correct stage.
  • Handling multiple Prism CSS files: Your method is great.

This is excellent work. Once you make that small change, this PR will be ready to merge. Thanks again!

@hemanth5055
Copy link
Contributor Author

Hey @dineshsutihar, thanks a lot for your guidance! 🙌 I’ve fixed the code, and it’s ready to be merged now.

@dineshsutihar
Copy link
Collaborator

@hemanth5055 Good Work. Let me test it and then we will merge it.

Copy link
Collaborator

@dineshsutihar dineshsutihar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dineshsutihar dineshsutihar merged commit c174350 into StructZ:develop Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants