vikunja/frontend
Archived
6
39
Fork 48
Code Issues 1 Pull Requests 4 Projects Releases 26 Activity

feat: use variable fonts with subsetting #2817

Merged
konrad merged 1 commits from dpschen/frontend:feature/variable-fonts-with-subsetting into main 2022-12-15 21:37:03 +00:00
Conversation 10 Commits 1 Files Changed 30 +331 -71
dpschen commented 2022-12-09 12:50:18 +00:00
Member
Copy Link

Allthough individual filesize of the variable fonts is larger, the total loaded filesize should be smaller than before.

Allthough individual filesize of the variable fonts is larger, the total _loaded_ filesize should be smaller than before.
dpschen added the
kind/bug
kind/feature
changes requested
area/internal-code
 labels 2022-12-09 12:50:18 +00:00
konrad was assigned by dpschen 2022-12-09 12:50:18 +00:00
frederick commented 2022-12-09 12:55:17 +00:00
Member
Copy Link

Hi dpschen!

Thank you for creating a PR!

I've deployed the changes of this PR on a preview environment under this URL: https://2817-feature-variable-fonts-with-subs--vikunja-frontend-preview.netlify.app

You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.

Have a nice day!

Beep boop, I'm a bot.

Hi dpschen! Thank you for creating a PR! I've deployed the changes of this PR on a preview environment under this URL: https://2817-feature-variable-fonts-with-subs--vikunja-frontend-preview.netlify.app You can use this url to view the changes live and test them out. You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/. Have a nice day! > Beep boop, I'm a bot.
dpschen Comment%!(EXTRA template.HTML=2022-12-09 13:01:58 +00:00)
src/components/home/navigation.vue Outdated
@ -435,7 +435,7 @@ $vikunja-nav-selected-width: 0.4rem;
.menu-list {
li {
font-weight: 500;
dpschen commented 2022-12-09 12:52:50 +00:00
Author
Member

It seems like our 500 file was actually a semi-bold, meaning 600.
When I changed to the variable font everything was a bit to thin.
Also the exported 500. That's the reason for this change.
One disadvantage of this: if there is no variable font loaded the browser might round this up to bold for the fallback font. Since we preload the font the probability should be very small that this happens.

It seems like our 500 file was actually a semi-bold, meaning 600. When I changed to the variable font everything was a bit to thin. Also the exported 500. That's the reason for this change. One disadvantage of this: if there is no variable font loaded the browser might round this up to bold for the fallback font. Since we preload the font the probability should be very small that this happens.
src/components/home/navigation.vue Outdated
@ -480,2 +480,4 @@
color: var(--grey-500);
margin-right: .5rem;
// align brackets with number
font-feature-settings: "case";
dpschen commented 2022-12-09 12:53:07 +00:00
Author
Member

This aligns the brackets with the number.

This aligns the brackets with the number.
👍 1
dpschen marked this conversation as resolved
src/components/namespace/namespace-settings-dropdown.vue Outdated
@ -98,5 +98,6 @@ function setSubscriptionInStore(sub: ISubscription) {
<style scoped lang="scss">
.dropdown-trigger {
padding: 0.5rem;
display: flex;
dpschen commented 2022-12-09 12:53:24 +00:00
Author
Member

Aligns the three dots with the rest of the line

Aligns the three dots with the rest of the line
👍 1
dpschen marked this conversation as resolved
src/components/tasks/partials/singleTaskInList.vue Outdated
@ -39,3 +39,3 @@
{{ pt.title }}<template v-if="(i + 1) < task.relatedTasks.parenttask.length">,&nbsp;</template>
</template>
>
&rsaquo;
dpschen commented 2022-12-09 12:53:48 +00:00
Author
Member

More fitting sign for the parent child relation indicator

More fitting sign for the parent child relation indicator
👍 1
dpschen marked this conversation as resolved
src/components/tasks/partials/singleTaskInList.vue Outdated
@ -72,3 +72,3 @@
:aria-expanded="showDefer ? 'true' : 'false'"
>
- {{ $t('task.detail.due', {at: formatDateSince(task.dueDate)}) }}
– {{ $t('task.detail.due', {at: formatDateSince(task.dueDate)}) }}
dpschen commented 2022-12-09 12:54:01 +00:00
Author
Member

Longer sign.

Longer sign.
👍 1
dpschen marked this conversation as resolved
src/styles/common-imports.scss Outdated
@ -10,13 +10,14 @@
// are defined here.
//
$family-sans-serif: 'Open Sans', Helvetica, Arial, sans-serif;
dpschen commented 2022-12-09 12:54:19 +00:00
Author
Member

This was the actual issue why Open Sans wasn't used.

This was the actual issue why Open Sans wasn't used.
konrad commented 2022-12-09 19:02:38 +00:00
Owner

Because the variable was defined after the bulma import?

Because the variable was defined after the bulma import?
dpschen commented 2022-12-09 21:37:23 +00:00
Author
Member

yes. by setting the variable before the variables in bulma dont fall back to their default

yes. by setting the variable before the variables in bulma dont fall back to their default
dpschen marked this conversation as resolved
src/styles/fonts.scss Outdated
@ -1,65 +1,105 @@
$font-files-path: '/fonts/';
$font-files-path: '@/assets/fonts/';
dpschen commented 2022-12-09 12:56:43 +00:00
Author
Member

By not using the public folder for the font files, a hash gets added to the final font file.
This should simplify caching, since we could cache forever.

This additional hash is needed in case we change something inside the instance_and_subset function that has nothing to do with the input arguments (original font file / parameters).

By not using the public folder for the font files, a hash gets added to the final font file. This should simplify caching, since we could cache forever. This additional hash is needed in case we change something inside the `instance_and_subset` function that has nothing to do with the input arguments (original font file / parameters).
👍 1
dpschen marked this conversation as resolved
src/styles/fonts.scss Outdated
@ -12,0 +3,4 @@
// this is the same as the latin range that google uses.
// see for examle the unicode-range definition here:
// https://fonts.googleapis.com/css2?family=Open+Sans
$unicode-range: "U+0000-00FF,U+0131,U+0152-0153,U+02BB-02BC,\
dpschen commented 2022-12-09 12:57:48 +00:00
Author
Member

The current unicode range only supports (what google defines as) 'latin'. We could extend that in the future and create more additional subsets.

The current unicode range only supports (what google defines as) 'latin'. We could extend that in the future and create more additional subsets.
👍 1
dpschen marked this conversation as resolved
src/styles/fonts.scss Outdated
@ -21,0 +11,4 @@
@font-face {
font-family: 'Quicksand';
src: url($font-files-path + 'Quicksand[wght]_87bdcc7f.woff2') format('woff2-variations');
src: url($font-files-path + 'Quicksand[wght]_87bdcc7f.woff2') format('woff2') tech('variations');
dpschen commented 2022-12-09 12:58:11 +00:00
Author
Member

This is the upcoming syntax. Recommended to use both currently

This is the upcoming syntax. Recommended to use both currently
👍 1
dpschen marked this conversation as resolved
src/styles/fonts.scss Outdated
@ -21,0 +15,4 @@
font-style: normal;
font-weight: 400 700;
font-display: swap;
unicode-range: $unicode-range;
dpschen commented 2022-12-09 13:01:54 +00:00
Author
Member

If I remember correctly the font files before were already subsetted. So this unicode-range definition basically just tells what's written in the file. It gets useful in the moment that we add multiple ranges.

If I remember correctly the font files before were already subsetted. So this unicode-range definition basically just tells what's written in the file. It gets useful in the moment that we add multiple ranges.
👍 1
dpschen marked this conversation as resolved
src/styles/fonts.scss Outdated
@ -30,0 +21,4 @@
@font-face {
font-family: 'Open Sans';
src: url($font-files-path + 'OpenSans[wght]_54a65da5.woff2') format('woff2-variations');
src: url($font-files-path + 'OpenSans[wght]_54a65da5.woff2') format('woff2') tech('variations');
dpschen commented 2022-12-09 13:00:15 +00:00
Author
Member

Every time we update the font we need to change the file name here. I don't have a better solution for this as of now. For sure I could have created this file via a script, but that seemed more complex than simply updating the hash. It doesn't happen that often that a new font version is released. But it's good that in that case we are prepared.

Every time we update the font we need to change the file name here. I don't have a better solution for this as of now. For sure I could have created this file via a script, but that seemed more complex than simply updating the hash. It doesn't happen that often that a new font version is released. But it's good that in that case we are prepared.
👍 1
dpschen marked this conversation as resolved
dpschen requested review from konrad 2022-12-09 13:02:06 +00:00
konrad commented 2022-12-09 19:06:00 +00:00
Owner
Copy Link

Looks fancy!

Not sure if I understood everything so here's my recap:

  • We're now using variable fonts instead of pre-bundled fonts which allows for smaller font sizes and less downloads on newer browsers
    -> Is it backwards compatible?
  • There are scripts to download and parse new versions of the fonts but the actual font files are still kept in the repo.

Is that correct?

Looks fancy! Not sure if I understood everything so here's my recap: * We're now using variable fonts instead of pre-bundled fonts which allows for smaller font sizes and less downloads on newer browsers -> Is it backwards compatible? * There are scripts to download and parse new versions of the fonts but the actual font files are still kept in the repo. Is that correct?
dpschen commented 2022-12-09 21:50:50 +00:00
Author
Member
Copy Link

Looks fancy!

  • We're now using variable fonts instead of pre-bundled fonts which allows for smaller font sizes and less downloads on newer browsers

the smaller font size is a neat side effect. yes. variable fonts allow us also to do stuff like animating the font weight.

-> Is it backwards compatible?

yes. we also still create static fallbacks. see the fonts.scss. must browsers should support it by now though.

  • There are scripts to download and parse new versions of the fonts but the actual font files are still kept in the repo.

Is that correct?

I places the original font files inside the repo. by original I mean in this case: the variable font without any change. I decided to do this as a backup in case the download isn't possible in the future out of whatever reasons. It's really just a precaution.

The instantiation (dropping of axis and / or their range) and subsetting happens on top of the downloaded font.

Does it work for you?
In theory it shouldn't change the font files since the hash should stay the same.

I tried to use as few dependencies as possible. Do you think a script in this form makes sense? Or should it run e.g. in a docker file.

> Looks fancy!  > * We're now using variable fonts instead of pre-bundled fonts which allows for smaller font sizes and less downloads on newer browsers the smaller font size is a neat side effect. yes. variable fonts allow us also to do stuff like animating the font weight. > -> Is it backwards compatible? yes. we also still create static fallbacks. see the fonts.scss. must browsers should support it by now though. > * There are scripts to download and parse new versions of the fonts but the actual font files are still kept in the repo. > > Is that correct? I places the original font files inside the repo. by original I mean in this case: the variable font without any change. I decided to do this as a backup in case the download isn't possible in the future out of whatever reasons. It's really just a precaution. The instantiation (dropping of axis and / or their range) and subsetting happens on top of the downloaded font. Does it work for you? In theory it shouldn't change the font files since the hash should stay the same. I tried to use as few dependencies as possible. Do you think a script in this form makes sense? Or should it run e.g. in a docker file.
konrad commented 2022-12-10 10:13:58 +00:00
Owner
Copy Link

I places the original font files inside the repo. by original I mean in this case: the variable font without any change. I decided to do this as a backup in case the download isn't possible in the future out of whatever reasons. It's really just a precaution.

That's a good idea. Also we'll avoid network traffic to external sources every time this is built in CI.

Does it work for you?

yes!

I tried to use as few dependencies as possible. Do you think a script in this form makes sense? Or should it run e.g. in a docker file.

I think this is fine, but the dependencies like python etc should be documented (or added to the nix flake)

> I places the original font files inside the repo. by original I mean in this case: the variable font without any change. I decided to do this as a backup in case the download isn't possible in the future out of whatever reasons. It's really just a precaution. That's a good idea. Also we'll avoid network traffic to external sources every time this is built in CI. > Does it work for you? yes! > I tried to use as few dependencies as possible. Do you think a script in this form makes sense? Or should it run e.g. in a docker file. I think this is fine, but the dependencies like python etc should be documented (or added to the nix flake)
dpschen commented 2022-12-10 10:17:59 +00:00
Author
Member
Copy Link

That's a good idea. Also we'll avoid network traffic to external sources every time this is built in CI.

I don’t see this as something that would run in the CI. More as sonneting you would run manually from time to time.

I think this is fine, but the dependencies like python etc should be documented (or added to the nix flake)

Considering the above: should it still be included? Send if so can you help me with that?

> That's a good idea. Also we'll avoid network traffic to external sources every time this is built in CI. I don’t see this as something that would run in the CI. More as sonneting you would run manually from time to time. > I think this is fine, but the dependencies like python etc should be documented (or added to the nix flake) Considering the above: should it still be included? Send if so can you help me with that?
konrad commented 2022-12-10 18:22:47 +00:00
Owner
Copy Link

Considering the above: should it still be included? Send if so can you help me with that?

If a Dockerfile makes it easier, yes please add one.

> Considering the above: should it still be included? Send if so can you help me with that? If a Dockerfile makes it easier, yes please add one.
dpschen commented 2022-12-13 11:37:21 +00:00
Author
Member
Copy Link

Considering the above: should it still be included? Send if so can you help me with that?

If a Dockerfile makes it easier, yes please add one.

It would make it more complex.

> > Considering the above: should it still be included? Send if so can you help me with that? > > If a Dockerfile makes it easier, yes please add one. It would make it more complex.
konrad commented 2022-12-13 19:08:31 +00:00
Owner
Copy Link

It would make it more complex.

Then it's fine to not include one. But please document what dependencies the script has (can be a comment in the header of the shell script).

> It would make it more complex. Then it's fine to not include one. But please document what dependencies the script has (can be a comment in the header of the shell script).
dpschen force-pushed feature/variable-fonts-with-subsetting from 159e064851 to 1da52d5bc1 2022-12-13 23:00:51 +00:00 Compare
dpschen commented 2022-12-13 23:02:45 +00:00
Author
Member
Copy Link

Then it's fine to not include one. But please document what dependencies the script has (can be a comment in the header of the shell script).

I guess the only real dependency is python 3. The required packages are installed here

I added instructions here.

The rest should be installed on most distributions (maybe not alpine, but 0815 ubuntu should work I guess).

Is that enough for you or were you expecting more?

PS: Next time I'll write it in python from the start.

> Then it's fine to not include one. But please document what dependencies the script has (can be a comment in the header of the shell script). I guess the only real dependency is python 3. The required packages [are installed here](https://kolaente.dev/vikunja/frontend/src/commit/1da52d5bc103d642d81907d30e40cef57628f140/scripts/fonts-subset.sh#L118) I added [instructions here](https://kolaente.dev/vikunja/frontend/src/commit/1da52d5bc103d642d81907d30e40cef57628f140/scripts/fonts-subset.sh#L17). The rest should be installed on most distributions (maybe not alpine, but 0815 ubuntu should work I guess). Is that enough for you or were you expecting more? PS: Next time I'll write it in python from the start.
👍 1
konrad merged commit b6a89a0cde into main 2022-12-15 21:37:03 +00:00
konrad deleted branch feature/variable-fonts-with-subsetting 2022-12-15 21:37:03 +00:00
dpschen commented 2022-12-15 23:06:02 +00:00
Author
Member
Copy Link

🥳

🥳
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
konrad
Labels
Clear labels   
area/internal-code

   
changes requested

   
confirmed

   
dependencies

   
duplicate

This issue or pull request already exists

  
good first issue

   
help wanted

Need some help

  
hosting

   
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
question

More information is needed

  
wontfix

This won't be fixed

No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
konrad
3 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/frontend#2817
No description provided.
Delete Branch "dpschen/frontend:feature/variable-fonts-with-subsetting"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?