Jieiku / abridge

Fast & Lightweight Zola Theme
https://abridge.pages.dev/
MIT License
142 stars 41 forks source link

Clearer instructions in readme and scss file #138

Closed Hysterelius closed 10 months ago

Hysterelius commented 10 months ago

First of all, I would like to praise you on the amazing work you have done on the abridge project. While using it, I came across a few minor issues and things you could clear up in the documentation with the newest version (I am using the latest commit).

I have had to create my own .scss file to import custom fonts into the project, like the Roboto.scss file. This could be made a little clearer.

https://github.com/Jieiku/abridge/blob/009d3f90b1881530bd28003abe83cfc3735882be/sass/fonts/_Roboto.scss#L1-L17

I also found that in order to prepend the fonts file, it is necessary to use null on lines 127-128 of the abridge.scss file which is undocumented.

https://github.com/Jieiku/abridge/blob/009d3f90b1881530bd28003abe83cfc3735882be/COPY-TO-ROOT-SASS/abridge.scss#L127-L128

Furthermore, while using the abridge switcher, I had to change the value of switcherDefault to light to make my code block formatting be of the same colour theme as the rest of my website. Is this the intended behaviour?

https://github.com/Jieiku/abridge/blob/009d3f90b1881530bd28003abe83cfc3735882be/COPY-TO-ROOT-SASS/abridge.scss#L13

I would be more than happy to contribute to this project by submitting a PR. Thank you for your time and consideration.

Jieiku commented 10 months ago

Thanks for using Abridge, and happy to hear you are getting up to date with the latest code. Thank you for pointing out the font issues, because I use the system font stack myself, I had not tested loading a font resource in a very long time.

Ah yes, null is used if you want to prepend the list instead of append. very good point will add this info. I am also going to add an example font to the COPY-TO-ROOT-SASS folder.

Furthermore, while using the abridge switcher, I had to change the value of switcherDefault to light to make my code block formatting be of the same colour theme as the rest of my website. Is this the intended behaviour?

No, definitely should not be doing that: you can see that in the demo it works properly: https://abridge.netlify.app/overview-code-blocks/

I suspect the issue here is cache related, try Ctrl+shift+P to open a private browsing session. If the issue persists then I would ask if the source code for your website is in a repo so that I could take a look.

Jieiku commented 10 months ago

I added examples and documentation for the fonts: https://github.com/Jieiku/abridge/commit/98ad16f5e48f11f8ad92717384253cb9b6ffe0c9

I am not going to close this issue yet though, because I am curious about your code blocks....

Let me know what your able to figure out with those, Thanks!

Abridge.css repository uses abridge as a submodule: https://github.com/Jieiku/abridge.css/tree/master/themes

You can see it's code page here: https://abridgecss.netlify.app/overview-code-blocks/

The code blocks change with the rest of the page. I am not seeing any issues.

EDIT: I overlooked your comment about the pull request, improvements are always welcome if they are in line with the goals of abridge. feel free to submit a pull request or open an issue report to discuss features anytime. I appreciate it!

Hysterelius commented 10 months ago

This what my website looks like when switcherDefault is set to light:

switcher-light-abridge

Then what it is like when switcherDefault is set to dark:

switcher-dark-abridge

Thanks for all your work on the documentation front.

Jieiku commented 10 months ago

I checked your github, does not seem your sites source is in a public repository.

If you want to upload your site to a repo minus your content and redact anything sensitive in config.toml then I could take a look and probably figure it out.

The first thing I would do if I had it in front of me is doing a diff between the root sass/abridge.scss as well as the config.toml with the ones provided by abridge.

I am interested in this not only to figure it out for you, but also others that may run into the same issue. It does not happen on a fresh install:

git clone https://github.com/Jieiku/abridge
cd abridge
zola serve
Hysterelius commented 10 months ago

I am more than happy to make the repo and make it public, but I am currently unable. Give me a couple of hours, and I'll have a look then :)

Hysterelius commented 10 months ago

I've just added you to my repo. I've found something very odd, when I run zola serve the website I get uses my custom abridge.scss but when I run zola build it uses the default abridge.scss.

EDIT: Also, when I ran npm run elasticlunr, I get an error saying it cannot find static/js/theme.js. Will I need to copy this across?

Jieiku commented 10 months ago

I was just headed to bed, planning to take a look tomorrow :) any files named the same locally should be used over the theme files, at-least that has been my experience so far.

Jieiku commented 10 months ago

I pushed a commit to your repo. You had sass directory in the .gitignore. Which might have been appropriate under the old abridge. Now however we use a scss overrides file for customization in that directory.

I also cleaned up some differences in the config.toml. copied the new sass overrides file in place, and added your customized colors and fonts to it.

All in all not too bad, only took like 20 minutes to fix any issues that I am able to see. Make sure to check it over good for any other customizations I might have missed.

hysterelius

It seems the npm scripts are currently tuned to work from abridge theme, I was working to minimize the number of entries at one point for the package.json. So I will have a look at that once I get a little bit more free time, probably in a day or two.

Have a great weekend.

Jieiku commented 10 months ago

One thing I noticed while putting your custom font in place, then using ctrl+f5 to refresh the page, is that the fonts appear to download twice in the developer console. I believe this is because of preload.

I just made a note to investigate this later. (the font is however working)

It may just be the way that preload declaration works, it seems navigation after they have been cached appears normal.

In the JetBrainsMono.scss file the local() part of the src is meant to load the font from the local operating system if it is already installed. If it is already installed on the computers, then it will skip downloading the file. The spelling here is actually really important for that feature to work.

When I was testing this for other fonts in the past, I would install the font to my system and figure out the expected font name, you may have noticed in the roboto font I declare a few local names for the roboto mono font. https://github.com/Jieiku/abridge/blob/master/sass/fonts/_Roboto-Mono.scss

When fonts have spaces in the name, it complicates things, even worse is if some OS/systems call the font something a little different than other OS/systems.

Hysterelius commented 10 months ago

Thank you for all your help, especially with fixing my repo. I am just cleaning up a few things and then I'll make it public. Thanks again for all your help :)

EDIT:

I pushed a commit to your repo. You had sass directory in the .gitignore. Which might have been appropriate under the old abridge. Now however we use a scss overrides file for customization in that directory.

Oops, yeah I should have realised that - I had a scss file but just forgot to push it to my repo (and remove it from the .gitignore.

If you point where to go, I am happy to try and help with the fonts or npm scripts.

Jieiku commented 10 months ago

oh no worries if you want to keep your site's source code private, I only made that comment previously because I was not sure how else to get access in order to look it over and help (had not occurred to me you could just add me as a user).

Hysterelius commented 10 months ago

No problem at all, I am more than happy to make it public - I just need to adjust a few things.

Thanks again, and if you would like me to help with anything let me know.

Jieiku commented 10 months ago

Thanks, I will let you know if I think of anything. I am planning to do the initial work on the npm scripts. I am going to do a total refactor: #140

Jieiku commented 10 months ago

I finished refactoring package.json. there is now really only one npm script to run, and it dynamically handles everything by first parsing your config.toml

npm run abridge

I will point out that if you were hoping to test tinysearch or stork you may have to wait a while, there seems to be an issue and I do not have time to investigate it: https://github.com/tinysearch/tinysearch/issues/175

I also tried stork again and ran into similar issues.

Elasticlunr is however working without issue.

Hysterelius commented 10 months ago

I have just tried to run npm run abridge and I have discovered just one or two minor issues. (I'm not sure if they are specific to my system or not)

I had to edit this line: https://github.com/Jieiku/abridge/blob/5e55301cc2c91a5f0e99000bfe4d7ec262ee78c1/package_abridge.js#L224

To be process.stdout.write(...., otherwise it threw an error for me.

Also, I had to copy across the manifest.json file, is it a default file or will I have to edit it?

Thanks for this awesome project!

Jieiku commented 10 months ago

Just pushed a commit that will check if the manifest.json file exists before attempting to minify it.

manifest.json is used by PWAs, it is what gives them a list of icons to use for various phones, tablets, etc.

If you open firefox developer console (ctrl+shift+I) then click application, then manifest, you can see it:

2023-09-08_22-41-18

Hysterelius commented 10 months ago

My website isn't a PWA, so I don't need a mainfest.json?

The npm script does error out if I don't have one.

EDIT: Just saw the commit, probably should have checked that before \:P

Jieiku commented 10 months ago

yeah, you should be fine without it if you set pwa = false in config.toml

did you pull the commit I just pushed? https://github.com/Jieiku/abridge/commit/9c6f4e078620b4d9d2ddf2ad17bdd3b97aa28704

Jieiku commented 10 months ago

Thanks for testing the latest changes, and the feedback!