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

Add support for additional fonts #17

Closed
simbleau opened this issue May 6, 2022 · 40 comments
Closed

Add support for additional fonts #17

simbleau opened this issue May 6, 2022 · 40 comments

Comments

@simbleau
Copy link
Contributor

simbleau commented May 6, 2022

No description provided.

@Jieiku
Copy link
Owner

Jieiku commented May 6, 2022

This is outside the goal of abridge which is to be lightweight and fast.

Actually I think I can add this in a way that it is disabled by default, so I can probably add a couple example fonts.

@simbleau
Copy link
Contributor Author

simbleau commented May 6, 2022

Awesome, thanks!

@Jieiku
Copy link
Owner

Jieiku commented May 6, 2022

Just added this. note the extra config.toml settings:

[extra]
# ...
# For externally loaded fonts make sure the include the FULL url including the http/https prefix
fonts_load = false
fonts = [
    {url = "https://fonts.googleapis.com/css?family=Source+Sans+Pro:400,700|Source+Code+Pro"},
    {url = "https://fonts.googleapis.com/css?family=Merriweather:300,400|Montserrat:400,700"},
    {url = "FiraCode-Bold.woff2"},
    {url = "FiraCode-Regular.woff2"},
]

to change which font is used edit:

:root {
// Typography
--ff: system-ui, -apple-system, "Segoe UI", Roboto, Ubuntu, Cantarell, "Noto Sans", sans-serif;
--fm: Menlo, Consolas, "Roboto Mono", "Ubuntu Monospace", "Noto Mono", "Oxygen Mono", "Liberation Mono", monospace;
--lh: 1.5; //line-height
--lhh: 1.2; //line-height headers
--fs: 1rem; //font-size
--fw: 400; //font-weight
--fh: 700; //font-weight h1-h6

@Jieiku Jieiku closed this as completed May 6, 2022
@eugenesvk
Copy link
Contributor

Is it possible to make the sass files use a variable with "!default" so that I can change your font list on import instead of having to edit the theme files directly?

@Jieiku
Copy link
Owner

Jieiku commented Apr 30, 2023

Ah, seems not having that breaks even overrides in the base site, I will add this asap to both the base theme and the refactor branch, as well as some of the other variables.

@eugenesvk
Copy link
Contributor

eugenesvk commented Apr 30, 2023

Thanks for the links, I've read them and use (though afaik it's better to use @use than @import per sass docs), but then it doesn't work properly for non-vars/mixins/functions, that is, for actual styles like root with the font-family, so even if I import your theme file as a module (@use "../../themes/abridge/sass/_variables.scss" as abridge-var) and then add my own family to root, I'll have two styles in the output, the original from import and my override (though in this case I don't understand the need for an import, I can just include the font-family string style directly and it will override your theme)

With a variable I could append my couple of fallback fonts to the end of your existing variable string (actually, not sure !default helps here as then I'd have to supply the whole list myself, but I can do module_name.$font_fam_var : string.insert(module_name.$font_fam_var, " my_fallback_font", -1)
And this

  • better shows intent (I don't want to change all the fonts, just add an extra symbol fallback font)
  • allows picking up whatever changes you make in the future theme versions since I'm not overwriting everything
  • does not duplicate styles in the output

@Jieiku
Copy link
Owner

Jieiku commented Apr 30, 2023

but I can do `module_name.$font_fam_var : string.insert(module_name.$font_fam_var, " my_fallback_font", -1)

oh, that is a nifty trick! I would like to try that and also add that to the documentation.

@Jieiku
Copy link
Owner

Jieiku commented Apr 30, 2023

I am trying to remember my reason for using a css variable instead of a sass variable. I assume that is the reason I am unable to add !default without affecting base theme font. Let me do a few tests real quick.

Pull requests are also welcome, if you have a change that you would like me to test, then merge.

@Jieiku
Copy link
Owner

Jieiku commented Apr 30, 2023

I think my only reason for using a css vairable --ff instead of a sass variable $ff was because sass variables can only be used within their scope.

At the time I was trying to define the font family within the vars file, I think one way to do this is to just set the sass variable in the _document.scss file.

Have you tried making any of these changes yet locally? I am assuming when you asked for a variable, you meant a sass variable instead of a css variable.

html {
text-rendering: optimizeLegibility;
background-color: var(--c1);
color: var(--f1);
font-family: var(--ff);
font-weight: var(--fw);
font-size: var(--fs);
line-height: var(--lh); // 1
// -ms-text-size-adjust: 100%; // 2
// -webkit-text-size-adjust: 100%; // 3
// -webkit-tap-highlight-color: rgba(0, 0, 0, 0); // 4
// cursor: default; // 5
// tab-size: 4; // 6
}

@eugenesvk
Copy link
Contributor

But it's not a sass variable, so you can't add !default there, I think it needs to be

$font-family-main : "Roboto", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", "Noto Sans", Helvetica, Arial, sans-serif !default
:root
  --ff: #{$font-family-main}  /* though you lose quotes here, which are important for fonts with numbers :((( */

So this would work unless fonts had numbers in them (like Noto Sans Symbols 2 fails, needs "Noto Sans Symbols 2", while regular fonts don't really need to be quoted)
Oh, well, strange sass interpolation rules kill this feature for this specific property

@Jieiku
Copy link
Owner

Jieiku commented Apr 30, 2023

ahhh I see what you did there, so it is still a css variable, but you set it using a sass variable.

Strange nobody has address the shortcoming of that font with numbers issue.... how common are fonts with numbers I wonder...

@eugenesvk
Copy link
Contributor

eugenesvk commented Apr 30, 2023

Have you tried making any of these changes yet locally? I am assuming when you asked for a variable, you meant a sass variable instead of a css variable.

Yes, I meant sass variables
I'm using them for dynamic color schemes within your theme, that's where you need a combo of both css vars and sass vars (sass vars allow using colors defined in some other sass modules, while css vars are used by your theme to set the switching dynamic)

:root.light
  --f1   	: #{var(--gray12)}	/* gray12 comes from another color theme */
  --tt-bg	: #{sol.$bg_l}    	/* $bg_l is from my local theme module file, but could also be an override from importing your theme files */

:root:not(.light)
  --f1   	: #{var(--gray12)}	/* gray12 var is from a color aware theme, so it stays the same as imports define the scope */
  --tt-bg	: #{sol.$bg_d}    	/* local theme defines two light/dark variables directly	*/

@Jieiku
Copy link
Owner

Jieiku commented Apr 30, 2023

Are you currently running the master branch of abridge or the refactor branch?

In the refactor branch I switched to using mixins for the colors.

This allowed me to change a theme's color profile in a single place instead of in the -auto, -dark, -light files

:root {
@include dark;
}

I am not sure yet when the Refactor branch will get merged into master. I still need to do more tests.

@eugenesvk
Copy link
Contributor

Strange nobody has address the shortcoming of that font with numbers issue

well, not really strange, these js/browser thingies are full of broken holes here and there that don't get fixed for years

how common are fonts with numbers I wonder

Not sure, but unfortunatly am using one, so my suggestion wouldn't help myself :( Or maybe it would, afaik you don't really need the css var as he font family is not dynamically changed with anything, right? So then I could just import your var, append it and use directly in

html {
  font-family: var(--ff);
}

Are you currently running the master branch of abridge or the refactor branch?

the master branch, saw the refactor efforts, tried it, but if I remember correctly would've needed to change a bit more than I liked at the time, so will wait till the new version is released

In the refactor branch I switched to using mixins for the colors.

nice, everything should be vars/functions/mixins so it can be adjused :)

(though I think it should be @use everywhere since it provides better encapsulation etc. https://sass-lang.com/documentation/at-rules/use#differences-from-import and @import is being phased out)

@Jieiku
Copy link
Owner

Jieiku commented Apr 30, 2023

I had considered using @use for the color mixin, but apparently it cannot be nested in style rules, so I am not sure how I would have made that work. There are probably a lot of other places I could be making use of @use though, so I will certainly add that to my todo list.

Also that is correct, the -ff variable simply gets set and applied to the html element, it never changes:

grep --color -i --binary-files=without-match --include=\* -rnw "/home/jieiku/.dev/abridge" -e "ff"

/home/jieiku/.dev/abridge/sass/include/_document.scss:28:  font-family: var(--ff);
/home/jieiku/.dev/abridge/sass/_variables.scss:117:  --ff: "Roboto", system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", "Oxygen", "Ubuntu", "Cantarell", "Fira Sans", "Droid Sans", "Helvetica Neue", "Noto Sans", Helvetica, Arial, sans-serif;

@eugenesvk
Copy link
Contributor

it cannot be nested in style rules, so I am not sure how I would have made that work

you can nest it with https://sass-lang.com/documentation/modules/meta#load-css (this has the nice feature of allowing to set names dynamically, not that it's a good idea :)), but the proper way from my shallow understanding is to group styles into mixins and then @include those in whatever nests you like by @include <namespace>.<mixin>()

@eugenesvk
Copy link
Contributor

I think I might have found a workaround for the quotation weirdness! You can use "'nested quotes'" to remove only the outer layer

@use "sass:string"
@use "sass:list"
@use "sass:meta"
// ↓ imported from your library
$mysassvar : "'Roboto'", "'Segoe UI'", "'Helvetica Neue'", sans-serif
// ↓ added in user code
$myextrafontlist: ("'my font 1'", "'my font 2'")
@debug $mysassvar
// ↓ or reverse order to prepend your fonts
$mysassvar : list.join($mysassvar, $myextrafontlist)
@debug $mysassvar

// ↓ only the "outer" quotes removed, 'inner' remain
.style
  --mycssvar : #{$mysassvar}

gets

.style {
  --mycssvar: 'Roboto', 'Segoe UI', 'Helvetica Neue', sans-serif, 'my font 1', 'my font 2';
}

@Jieiku
Copy link
Owner

Jieiku commented May 1, 2023

Nice, I started switching over to use instead of import, will give this a try very soon and implement it if I don't spot any issues. (just got home a little while ago, and busy day tomorrow too, but I usually code in the morning and evenings even when im busy)

@eugenesvk
Copy link
Contributor

Also found this trick in sass issues

or you can use interpolation in the property name so that Sass's special custom property parsing behavior (which is necessary for CSS compatibility) doesn't activate:

$font-family: "Font with Spaces and 33", serif
:root
  #{--font-family}: $font-family

@Jieiku
Copy link
Owner

Jieiku commented May 1, 2023

Awesome, that looks like a much cleaner solution.

@eugenesvk
Copy link
Contributor

Yeah, and you can use regular //comments in this format instead of the /* loud ones */ within regular --cssvars!

@Jieiku
Copy link
Owner

Jieiku commented May 1, 2023

I only use a handful of loud comments, usually at the top of files, are you proposing doing something different with those?

/**
* SVG Icons
* https://github.com/feathericons/feather
* https://github.com/tabler/tabler-icons/find/master
*
* https://yqnn.github.io/svg-path-editor/
* https://jakearchibald.github.io/svgomg/
* https://codepen.io/yoksel/pen/MWKeKK
*/
@function icon($icon-name,$stroke,$stroke-width: 1,$fill:'none'){

@eugenesvk
Copy link
Contributor

Wasn't proposing anything re. your code, if you think those need to be present in the output, that's fine.

I was just mentioning a benefit of the other notation since I got cut by the fact that css vars don't support regular comments and was wondering why the styles wouldn't apply...

@Jieiku
Copy link
Owner

Jieiku commented May 1, 2023

I merged your pull request, I also replaced all occurrences of import with use, except for the ones that involve the color mixins that I use, need to think about the cleanest way to do that.

@eugenesvk
Copy link
Contributor

eugenesvk commented May 2, 2023

The variables still define rules directly, so the abridge-var.$font trick doesn't yet work:

// userMainCustomFile.sass
@use "_variables" as abridge-var with ($font:("RobotoWith"))

abridge-var.$font : "RobotoVarOverride"
// ↑ this fails without a mixin, the output still be :root {--ff: "RobotoWith";}
// ↓ but if _variables didn't have rules directly, but instead encapsulated them in a mixin
@include abridge-var.all-rules
// ↑ this would output: :root {--ff: "RobotoVarOverride";}
// abridge's _variables.sass
$font: "RobotoOriginal" !default

// ↓1 currently, no mixin
:root
  #{--ff}: $font // ← this can only be overriden with `@use "_variables" with($font:"RobotoWith")`
// but then you can't append anything, only replace

// ↓2 bright future, mixin, can dynamically update with `abridge-var.$font`
@mixin all-rules
  :root // side note: though if it's a mixin, maybe it should not have a selector and let the caller set it?
    #{--ff}: $font

(as an illustration: this allows me to override it in my theme as described above 43d36cd)

@eugenesvk
Copy link
Contributor

I merged your pull request, I also replaced all occurrences of import with use, except for the ones that involve the color mixins that I use, need to think about the cleanest way to do that.

I think the color mixins just need to use @forward

// imports.sass
@forward "colors/orange" as theme-*

and then you can use it like so:

// abridge.sass
@use "imports" as i
:root.light
  @include i.theme-light

Although trying to use @forward lead to grass sass compiler panicking, not sure what's going on there, that's up to grass/zola

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

I am seeing the same problem.

I am going to setup a VERY simple test case for this in a repo and open an issue at the grass repository. it appears in zola's cargo.lock file that they are using the latest grass crate, also this version should support @forward so I am not sure what the problem is.

https://github.com/getzola/zola/blob/2b56bcb967cb7284e3225fc5caac50b095185004/Cargo.lock#L1191-L1198

@Jieiku Jieiku reopened this May 2, 2023
@eugenesvk
Copy link
Contributor

wanted to post it in the other issue you opened, but that was deleted too quickly :)

FYI this is the appending function I use

// file: _function.sass
@use "sass:meta"
@use "sass:list"

@function append-font($base, $ext, $font-sep)
  $base-pre	: ()
  $base-pos	: ()
  $iFontSep : index($base, $font-sep)
  @if not $iFontSep // didn't find the font separator, return
    @return $base

  @for $i from 1 through list.length($base)
    $iEl: list.nth($base, $i)
    @if $i <= $iFontSep
      $base-pre: list.append($base-pre, $iEl, $separator:comma)
    @else
      $base-pos: list.append($base-pos, $iEl, $separator:comma)

  @return list.join(list.join($base-pre, $ext), $base-pos)

and how you could use it in the main file

// file: main.sass
@use "../path/to/../themes/abridge/sass/_variables"	as var
@use "function"                                    	as f
// this appends two fonts after Segoe UI in the main font family and Segoe UI Mono in the code font family
$iSegoe-Main     	: index(var.$font     , "Segoe UI"     ) // find index within existing variable
$iSegoe-Code     	: index(var.$font-mono, "Segoe UI Mono")
$font-ext-symbols	: ("Noto Sans Symbols 2", "Noto Sans Symbols") // fonts to append
@if $iSegoe-Main // if font not found, silently do nothing
  var.$font      : f.append-font(var.$font     , $font-ext-symbols, "Segoe UI"     )
@if $iSegoe-Code
  var.$font-mono : f.append-font(var.$font-mono, $font-ext-symbols, "Segoe UI Mono")

@eugenesvk
Copy link
Contributor

According to this grass issue connorskees/grass#19 forward is indeed supposed to be supported

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

yep I am going to setup a barebones repo of a zola theme for them to test this. It will basically be a single page with a couple sass files, something much simpler than abridge that easily identifies the problem.

Issues get fixed quicker when you can give a very simple example of the problem.

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

LOL! and this is why you make simple test cases, my simple test case the feature works without issue:

git clone https://github.com/Jieiku/zola-grass-forward-test
cd zola-grass-forward-test
cd fails-with-forward
zola serve

2023-05-02_10-16-17

Now we have to figure out why.

The only thing I can think to do is slowly introduce more of abridge's sass files until the error triggers.

That should give us and the grass developers more of an idea of the cause.

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

There is now zero uses of import, there must have been a typo before... I am no longer having an issue with forward... only problem is I am not sure what I did differently.

Can you do a git pull on the refactor branch and see if it is the same for you.

@eugenesvk
Copy link
Contributor

it was due to the imports at dark/light themes, for example

//abridge-dark.scss
// panic
@import "imports";
@import "syntax/syntax-abridge-dark";

:root {
  @include dark;
}

// no panic
@use "imports"                   	as i;
@use "syntax/syntax-abridge-dark"	as syntax;

:root {
  @include i.dark;
}

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

so it was simply that all 4 needed to be switched over at once, I had wondered that. All the imports are gone now.

I am going to review your function, etc for appending the font.

That is a feature that I would like to be available for everyone that uses Abridge.

If you have tested it all out then feel free to submit a pull request, and I will review it asap.

If you don't have time then just let me know and I can try to implement it on my end from your example.

@eugenesvk
Copy link
Contributor

yeah, and if you switch them to mixins, you could have all the syntax colors defined in one file in two light/dark mixins and then just use the relevant one in the abridge-light/dark/auto file

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

The colors currently are set to mixins:

@mixin dark {

so I am assuming you mean something different?

OH! your saying have all the colors in a single file...

@eugenesvk
Copy link
Contributor

I meant the syntax colors
syntax-abridge-auto
syntax-abridge-dark
syntax-abridge-light

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

ahhh yes, your absolutely right, that would be a good idea.

I actually had that on my todo list, but had not done it yet, Keats is reviewing swapping out syntect eventually, should probably still go ahead and move them to mixins though for the time being:

https://github.com/getzola/zola/issues/1787

I will work on moving the syntax to mixins this evening.

@eugenesvk
Copy link
Contributor

eugenesvk commented May 2, 2023

swapping out syntect eventually

ah, ok (though I dislike TextMate grammar mentioned there as a potential replacement, Sublime's (the new version) is much more ergonomic)

Re the fonts - what did you have in mind re. adding it for everyone,

  • would users edit some sass script directly in your theme or
  • copy&paste an example to their main file and tweak it there?

I'm going down the 2nd route (and actually disable loading the theme's files in with stylesheets = [ ] to avoid having duplication in the theme's file and my overrides), so in that sense the function is tested and does add those two fonts to the mix

(and zola doesn't expose any config vars in the sass processing, right?, so the users can't set extra font family names in the config, there are no sass tera templates?)

@Jieiku
Copy link
Owner

Jieiku commented May 2, 2023

2nd route sounds perfect

That is correct you cannot tera template a sass file yet. I am actually trying to extend zola's ability to template additional files, at the moment js and json files, but i am still very new to rust (so it will take me a bit with my limited available time):

https://github.com/Jieiku/zola/commit/995a0d39ac96b7a93ea2a8f52862062c2c8775ce

getzola/zola#2167

@Jieiku Jieiku closed this as completed May 11, 2023
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

No branches or pull requests

3 participants