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

Allow using all of github's themes #101

Merged
merged 2 commits into from
Apr 17, 2023
Merged

Allow using all of github's themes #101

merged 2 commits into from
Apr 17, 2023

Conversation

jjspace
Copy link
Contributor

@jjspace jjspace commented Apr 16, 2023

2023-04-16_17-02

Summary

  • Utilize the updates added to generate-github-css to include options for all theme types in a new generation script
  • Modify the included stylesheets to apply based off of data attributes instead of a single class name to allow for easier mixing and matching. (This matches up closely with how Github themselves does it)
  • Expand logic for selecting a theme to allow specifying the light and dark themes to use when in Auto, System and Single modes
  • Add settings for Light an Dark themes separate from one main theme setting
  • Changed project over to a type: module to allow working with generate-github-markdown which is an ESM module not a CommonJS module. This also required changing the extension.js to extension.cjs to continue working as it used to
  • Updated the Readme to reflect the new settings (and also address a question I saw in the issues list)

I believe the way the new settings are set up will preserve the existing behavior for anyone who had modified them before.

Resolves #89
Resolves #75

Please let me know if you have any questions!

I don't know much about actually publishing extensions so I believe all my changes should work for those who install them but please verify for me so I'm not accidentally breaking this for a bunch of users (myself included)

@mjbvz mjbvz merged commit 4c8fac2 into mjbvz:master Apr 17, 2023
@mjbvz
Copy link
Owner

mjbvz commented Apr 17, 2023

Great work! Looks like a very nice addition. Will test a little more and then publish shortly if everything looks good

@jjspace jjspace deleted the more-themes branch April 17, 2023 21:21
@jjspace
Copy link
Contributor Author

jjspace commented Apr 19, 2023

@mjbvz I'm sorry for accidentally creating some extra issues with my changes and making you do extra cleanup (#104, #103). Can you elaborate a little on why your fixes were needed/the better changes in these cases? (e0286fd b2adb1f, d01f1e7) Just so I can learn and don't repeat mistakes in the future as I've never worked on vscode extensions before (and "it works fine on my machine" is not a great excuse 😆)

ps. using this pr just to avoid excess noise for other users in the issues, wasn't sure the best place for a discussion

@mjbvz
Copy link
Owner

mjbvz commented Apr 19, 2023

No worries, the fixes were pretty quick and easy (and the more important break was my fault)

To recap. The first issue was that .cjs doesn't seem to work on web. I fixed that but in the process broke this extension on desktop because I forgot to commit the change that also removed type: "module" in the package.json. type: "module" caused the extension.js script to be loaded as a module, which was now failing because it's commonjs and no longer had a explicit .cjs extension to tell node otherwise. Then finally I had to make sure the build script was treated as a module again by converting to an .mjs file

Probably not a common issue. Will see if I can add a browser debug config to the extension so you can more easily test the extension in browsers

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.

Add Dark-dimmed mode High-contrast theme
2 participants