Skip to content

Conversation

@nadavtzaysler
Copy link
Contributor

@nadavtzaysler nadavtzaysler commented Jun 18, 2025

Description

added css file to fetch all used fonts from fixed path in app
changed dockerfile to take all used fonts from fonts.gstatic and load them into app
changed html to get css from file instead of google API

Fixes #26031

Additional context and related issues

Release notes

( X) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

## Section
* Fix Slowness in webserver. ({issue}`26031`)

@cla-bot
Copy link

cla-bot bot commented Jun 18, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@github-actions github-actions bot added the ui Web UI label Jun 18, 2025
@ebyhr ebyhr requested a review from mosabua June 19, 2025 02:53
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

Are you going to send a separate PR for the preview UI or is it all sorted out there already?

@cla-bot
Copy link

cla-bot bot commented Jun 19, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@cla-bot
Copy link

cla-bot bot commented Jun 19, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@nadavtzaysler
Copy link
Contributor Author

Are you going to send a separate PR for the preview UI or is it all sorted out there already?
The preview UI does not suffer from this issue, however I will open a new pr for the docs which also becomes slower because of the same issue

@mosabua
Copy link
Member

mosabua commented Jun 20, 2025

How does the current approach actually help? You are still not locally embedding the font files .. you are just using a CSS file now to point to them?

@cla-bot
Copy link

cla-bot bot commented Jun 22, 2025

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@mosabua
Copy link
Member

mosabua commented Jun 23, 2025

Thanks for actually adding the fonts.. before you just changed to a different URL from which to load the files and lead them from a separate CSS file .. now the files are actually local.

Please rebase and test this with different browsers and check on the browser console that no other fonts are requested

@mosabua
Copy link
Member

mosabua commented Jun 23, 2025

Also squash commits and update the commit message to

Localize fonts for web ui in the application

- Enable running UI without internet access and loading fonts from Google

@wendigo
Copy link
Contributor

wendigo commented Jun 23, 2025

Superseded by #26053

@wendigo wendigo closed this Jun 23, 2025
@nadavtzaysler
Copy link
Contributor Author

nadavtzaysler commented Jun 23, 2025

Why was this pr superseded?

@nadavtzaysler
Copy link
Contributor Author

@mosabua Can this pr be reopened, the pr that was supposed to supersede it doesn't seem more ideal and it's stale while this could've been merged

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

Labels

ui Web UI

Development

Successfully merging this pull request may close these issues.

Using Trino without internet connection will cause slowness in the UI

3 participants