swift-doc icon indicating copy to clipboard operation
swift-doc copied to clipboard

Published 20 hours ago • verified publisher icon SwiftDocOrg

Dark theme

Open literalpie opened this issue 3 years ago • 8 comments

I welcome feedback on anything, specifically:

  • Position and style of theme picker
  • The use of TypeScript - I chose it because I'm familiar and like type safety, but I can easily remove it
  • Fetching the JS from Swift. Based on conversations in other threads, it sounds like you have security concerns with the approach I'm using. If the SPM 5.3 resources feature will work for this, maybe it's worth waiting for that release before merging.

Hope you find this helpful!

literalpie avatar Aug 02 '20 22:08 literalpie

Nice work, @literalpie! I look forward to checking this out in the next day or two. In the meantime, and for posterity, would you mind sharing a few screenshots here?

mattt avatar Aug 03 '20 13:08 mattt

Sure, here you go! Screen Shot 2020-08-03 at 6 11 15 PM Screen Shot 2020-08-03 at 6 11 31 PM

literalpie avatar Aug 03 '20 22:08 literalpie

I've added changes to make use of SPM resource bundling allowed in 5.3. It references an in-progress PR to update SwiftSyntaxHighlighter, so this should be considered WIP until at least that is merged, and probably a little longer if we want to wait before requiring 5.3.

If the update to 5.3 should be it's own PR, let me know and I can split it out, or anyone can use my changes as a starting point.

literalpie avatar Sep 20 '20 16:09 literalpie

@literalpie Really nice work! Glad this is on track.

I was wondering, was omitting the color for main a deliberate decision?

screenshot-2020-10-17-2

kaishin avatar Oct 17 '20 19:10 kaishin

@kaishin There were already dark mode colors in the css file and I did not make any changes. Notice here that system-background and system-grouped-background are the same in dar mode, but different in light mode. I'm not sure what the reason for this is.

literalpie avatar Oct 17 '20 20:10 literalpie

@literalpie Right, I noticed after my comment. In that case that could be changed later.

@mattt Thoughts? I can do a pass on the colors after this PR is merged if you wouldn't mind.

kaishin avatar Oct 17 '20 20:10 kaishin

Matt, I've updated with support for both pre and post-5.3, the build is passing, and I have squashed my changes into one commit. I've also added logic to gracefully fall back when color-scheme queries are not supported, or when CSS custom properties are not supported.

Having made these changes, as far as I know, this is ready to merge. Let me know if you have any suggestions.

literalpie avatar Oct 17 '20 20:10 literalpie

Hi @mattt I'm still interested in getting this merged. Is now a good time to rebase, or are there other changes you are planning to get done before this is merged?

I see that you've moved the CSS from being a bundled file to a string in a Swift file. Should I do something similar with the JS generated here?

literalpie avatar Jul 31 '21 13:07 literalpie