search

dbarenholz / halcyon-obsidian

The Halcyon colour scheme by Brittany Chiang, ported to be used in Obsidian.
MIT License
20 stars 2 forks source link

Refactor to allow publish support #11

Closed sigrunixia closed 5 days ago

sigrunixia commented 5 months ago

Lets fix this thing up!

This will close #10

dbarenholz commented 5 months ago

Hi Sig!

Can you please split this up into the following PRs?

  1. Addition of publish support. Also think about modifying the build script so it automatically builds when running npm run build.
  2. Addition of stylelint, so that I can modify whatever stylelint wants, if desired.
  3. After (2) is merged: the refactor (potentially - I still have a hard time looking at the 3000+ lines changed).

Thank you!

dbarenholz commented 5 months ago

Also: any reason to bump the obsidian version? Is the theme not compatible with 1.0.0 anymore?

sigrunixia commented 5 months ago

Also: any reason to bump the obsidian version? Is the theme not compatible with 1.0.0 anymore?

We're going to be updating the sample templates to start at 1.5., so doing this for parity.

dbarenholz commented 5 months ago

Also: any reason to bump the obsidian version? Is the theme not compatible with 1.0.0 anymore?

We're going to be updating the sample templates to start at 1.5., so doing this for parity.

https://docs.obsidian.md/Plugins/User+interface/Right-to-left it appears we'll have to put it at 1.6 and make it RTL compatible :)

sigrunixia commented 4 months ago

If I recall correctly, we actually do not have any of the "faults" in Halcyon.

sigrunixia commented 4 months ago

I also suggest we do a gentle refactor first. If Publish goes first, I'd have to redo the work again.

Here is what ebullient and I did for splitting the files up into Publish and not-publish. Would this work for you?

https://github.com/ebullient/obsidian-theme-ebullientworks/tree/main/src/fragments

dbarenholz commented 4 months ago

I also suggest we do a gentle refactor first. If Publish goes first, I'd have to redo the work again.

Here is what ebullient and I did for splitting the files up into Publish and not-publish. Would this work for you?

https://github.com/ebullient/obsidian-theme-ebullientworks/tree/main/src/fragments

The idea: yes. The implementation of the idea (so a single fragments folder, and all files being dumped in there): no.

I want to keep separate files according to the documentation as listed in https://docs.obsidian.md/Reference/CSS+variables/CSS+variables.

I'm happy to refactor first; does the fragment-idea work while keeping separate files according to the link above?

dbarenholz commented 4 months ago

@sigrunixia Many comments in the review are effectively "please move a line up or down" -- there's only like 3 instances that require thought.

sigrunixia commented 5 days ago

Closing to resubmit, because editing existing PR when your branch is gone is annoying.