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.

Sign up for GitHub

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

Jump to bottom

Move source-code fonts to new build workflow #13093

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Move source-code fonts to new build workflow #13093

wants to merge 9 commits into from

Conversation

meel-hd
Copy link
Collaborator

@meel-hd meel-hd commented Nov 21, 2024

Proposed changes

Moved source-code fonts to new build workflow

Changes Include:

  • Removed the old scripts/yarn/** dir, as it is no longer needed.
  • Moved source-code-pro and source-sans to client/
  • Built and bundled the fonts.
  • Updated templates to use the new built fonts and styles.
  • Added new built bundles license info in REUSE.toml.

Note:

  • The old bundles did not get removed to keep this PR simple for review.

TODO:

  • Remove old fonts bundles.
  • Update any dependents on them to use the new bundles.
  • if FONTS_CDN_URL is configured it should be seted up with static/js/vendor/fonts instead of the old font-source/
  • Remove the old yarn-update script and its configs and workflows

Related: #12172

Checklist

  • Lint and unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added documentation to describe my feature.
  • I have squashed my commits into logic units.
  • I have described the changes in the commit messages.

Other information

@meel-hd
Move source-code fonts to new build workflow
1d1dd20 
Changes Include:
- Removed the old scripts/yarn/** dir, as it is no longer needed.
- Moved source-code-pro and source-sans to client/
- Built and bundled the fonts.
- Updated templates to use the new built fonts and styles.
- Added new built bundles license info in REUSE.toml.

Note:
- The old bundles did not get removed to keep this PR simple for review.

TODO:
- Remove old fonts bundles.
- Update any dependents on them to use the new bundles.
- if FONTS_CDN_URL is configured it should be seted up with static/js/vendor/fonts instead of the old font-source/
- Remove the old yarn-update workflow and its configs and workflows

Related: WeblateOrg#12172
@meel-hd meel-hd requested a review from nijel November 21, 2024 10:06
nijel
nijel reviewed Nov 21, 2024
View reviewed changes
weblate/static/js/vendor/fonts/fonts.css Outdated Show resolved Hide resolved
weblate/templates/snippets/meta-css.html Outdated Show resolved Hide resolved
scripts/yarn-update Outdated Show resolved Hide resolved
weblate/static/js/vendor/fonts/font-source/07e9568bc36099d13146.woff2 Outdated Show resolved Hide resolved
nijel added a commit to WeblateOrg/meta that referenced this pull request Nov 21, 2024
@nijel
chore: skip yarn-update if not present
3c66f2f 
See WeblateOrg/weblate#13093
nijel added a commit that referenced this pull request Nov 25, 2024
@nijel
chore(docs): fail on non-existing font directory
425131e 
This avoids silent breakages on changes like #13093.
nijel
nijel reviewed Nov 25, 2024
View reviewed changes
Copy link
Member

@nijel nijel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs/conf.py also needs to be changed to use the new location of the fonts. I've made the docs build fail when the fonts directory doesn't exist in 425131e.

client/webpack.config.js Outdated Show resolved Hide resolved
@meel-hd
use font names instead of hash
c7e2c10 
- Update docs/conf.py
- Update test_selenium.py
@meel-hd
Copy link
Collaborator Author

meel-hd commented Nov 26, 2024

@nijel I rerun the test suite on main (without the new changes) and it seems they fail also in there, I'm not sure what to do next.

summary

weblate/trans/tests/test_selenium.py::SeleniumTests::test_add_component missing element #list-add-button
weblate/trans/tests/test_selenium.py::SeleniumTests::test_fonts missing element #upload_font_submit
weblate/trans/tests/test_selenium.py::SeleniumTests::test_login missing element #dashboard-return
weblate/trans/tests/test_selenium.py::SeleniumTests::test_screenshots missing element #query-dropdown
weblate/trans/tests/test_selenium.py::SeleniumTests::test_weblate

nijel
nijel reviewed Nov 28, 2024
View reviewed changes
weblate/trans/tests/test_selenium.py
"font-source",
"TTF",
"SourceSans3-Bold.ttf",
"SourceSans3-Bold.ttf.woff2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is why the tests fails:

Suggested change
"SourceSans3-Bold.ttf.woff2",
"SourceSans3-Bold.ttf",

nijel
nijel reviewed Nov 28, 2024
View reviewed changes
docs/conf.py
"weblate/static/js/vendor/fonts/font-source/",
"weblate/static/vendor/font-kurinto/",
)
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a wrong merge, you should adjust font_locations above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nijel nijel nijel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@meel-hd @nijel