feat: self host google fonts by berzerkeer · Pull Request #22129 · appsmithorg/appsmith

berzerkeer · 2023-04-05T09:22:39Z

Description

22005

An air-gapped instance most likely can't communicate to the public internet, relying on google to load fonts is not possible.
So, this PR downloads and the required fonts on our local and serve them to the browser locally.

Fixes #22005
Fixes #22060

if no issue exists, please create an issue and ask the maintainers about this first

Media

A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Manual

Checklist:

Dev activity

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
PR is being merged under a feature flag

QA activity:

Test plan has been approved by relevant developers
Test plan has been peer reviewed by QA
Cypress test cases have been added and approved by either SDET or manual QA
Organized project review call with relevant stakeholders after Round 1/2 of QA
Added Test Plan Approved label after reveiwing all Cypress test

…irgap-image-bundle

…fonts folder

jsartisan · 2023-04-06T05:06:55Z

/ok-to-test sha=efe7c48

github-actions · 2023-04-06T05:07:24Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4625640911.
Workflow: Appsmith External Integration Test Workflow.
Commit: efe7c48.
PR: 22129.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=22129&runId=4625640911_1

github-actions · 2023-04-06T11:10:19Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4625640911.
Commit: efe7c48.
The following are new failures, please fix them before merging the PR:

cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Basic_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Theme_FormWidget_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Theme_MultiSelectWidget_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Button/ButtonGroup_MenuButton_Width_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Chart/Chart_spec.js
cypress/integration/Regression_TestSuite/ServerSideTests/MySQL_Datatypes/False_Spec.ts

Identified Flaky tests

github-actions · 2023-04-06T12:56:05Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4625640911.
Commit: efe7c48.
The following are new failures, please fix them before merging the PR:

cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Basic_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Theme_FormWidget_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Theme_MultiSelectWidget_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Button/ButtonGroup_MenuButton_Width_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Chart/Chart_spec.js

Identified Flaky tests

…undle-fonts

…nto feat/bundle-fonts

berzerkeer · 2023-04-10T07:36:24Z

/ok-to-test sha=a03ca99

github-actions · 2023-04-10T07:36:51Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4655562109.
Workflow: Appsmith External Integration Test Workflow.
Commit: a03ca99.
PR: 22129.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=22129&runId=4655562109_1

github-actions · 2023-04-12T07:02:26Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4675542765.
Workflow: Appsmith External Integration Test Workflow.
Commit: 336606c.
PR: 22129.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=22129&runId=4675542765_1

berzerkeer · 2023-04-12T07:18:11Z

/ok-to-test sha=15e51fe

github-actions · 2023-04-12T07:18:38Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4675678523.
Workflow: Appsmith External Integration Test Workflow.
Commit: 15e51fe.
PR: 22129.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=22129&runId=4675678523_1

jsartisan

LGTM!

app/client/src/pages/AppViewer/index.tsx

ichik · 2023-04-12T08:45:16Z

app/client/src/assets/fonts/google/index.css

@@ -0,0 +1,9 @@
+@import url("https://fonts.googleapis.com/css2?family=Nunito+Sans:ital,wght@0,300;0,400;0,700;1,300;1,400;1,700&display=swap");


Potential performance optimization for the future. Loading CSS with <link> is more performant than doing @import (ref).
Additionally, since the font is defined in the application theme we don't need to load everything in the deployed app.

github-actions · 2023-04-12T08:51:34Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4675678523.
Commit: 15e51fe.
The following are new failures, please fix them before merging the PR:

cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Theme_FormWidget_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Button/ButtonGroup_MenuButton_Width_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Chart/Chart_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/ListV2/Listv2_dataIdentifierProperty_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Sliders/CategroySlider_spec.ts

To know the list of identified flaky tests - Refer here

github-actions · 2023-04-12T12:04:00Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4675678523.
Commit: 15e51fe.
The following are new failures, please fix them before merging the PR:

cypress/integration/Regression_TestSuite/ClientSideTests/ThemingTests/Theme_FormWidget_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Button/ButtonGroup_MenuButton_Width_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Chart/Chart_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/ListV2/Listv2_dataIdentifierProperty_spec.js

To know the list of identified flaky tests - Refer here

berzerkeer · 2023-04-12T12:26:54Z

/ok-to-test sha=b216eea

github-actions · 2023-04-12T12:27:18Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4678266206.
Workflow: Appsmith External Integration Test Workflow.
Commit: b216eea.
PR: 22129.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=22129&runId=4678266206_1

berzerkeer · 2023-04-12T13:55:12Z

/ok-to-test sha=6c346f2

github-actions · 2023-04-12T13:55:36Z

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4679132691.
Workflow: Appsmith External Integration Test Workflow.
Commit: 6c346f2.
PR: 22129.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=22129&runId=4679132691_1

github-actions · 2023-04-12T15:32:59Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4679132691.
Commit: 6c346f2.
The following are new failures, please fix them before merging the PR:

cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Button/ButtonGroup_MenuButton_Width_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/TableV2/Inline_editing_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Workspace/DeleteWorkspace_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Workspace/MemberRoles_Spec.ts
cypress/integration/Regression_TestSuite/ServerSideTests/GenerateCRUD/Postgres2_Spec.ts
cypress/integration/Regression_TestSuite/ServerSideTests/OnLoadTests/JSOnLoad1_Spec.ts

To know the list of identified flaky tests - Refer here

github-actions · 2023-04-12T15:59:13Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4679132691.
Commit: 6c346f2.
The following are new failures, please fix them before merging the PR:

cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Button/ButtonGroup_MenuButton_Width_spec.js

To know the list of identified flaky tests - Refer here

berzerkeer · 2023-04-12T16:13:59Z

/ci-merge-check

github-actions · 2023-04-12T16:14:31Z

found ci-merge-check 22129

github-actions · 2023-04-12T17:02:01Z

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4679132691.
Commit: 6c346f2.
The following are new failures, please fix them before merging the PR:

cypress/integration/Regression_TestSuite/ClientSideTests/Widgets/Button/ButtonGroup_MenuButton_Width_spec.js

To know the list of identified flaky tests - Refer here

## Description Google fonts are served by the appsmith instance and change that was introduced in t[his PR](#22129). This PR removes redundant the import of google fonts from googleapis. Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### 🔍 Cypress test results  > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/10270353461> > Commit: 142bcc8 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10270353461&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Tue, 06 Aug 2024 17:21:17 UTC  ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No  ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Transitioned to a fully local/custom font strategy, enhancing branding consistency and potentially improving load times. - **Usability Improvements** - Enhanced output readability in the GitHub Actions workflow, making it easier for users to identify changed files. - Updated the file change detection mechanism in the GitHub Actions workflow for better performance and clarity.

Sangeeth Sivan added 10 commits April 2, 2023 23:18

feat: created a codesplitted image loader component for airgap

ec25975

feat: replace assets url with constant

3e275fb

feat: update regex

ed426ce

feat: replace and rename with AssetLoader

adcea65

feat: keep it simple to support multiple asset types

aa9d35e

chore: rename helper file and script file

51a917f

Merge branch 'release' of github.com:appsmithorg/appsmith into feat/a…

d4dee88

…irgap-image-bundle

Merge branch 'release' of github.com:appsmithorg/appsmith into feat/a…

8e4f60a

…irgap-image-bundle

fix: missed wrapping with util function

847c307

feat: self host google fonts

5e18d09

berzerkeer changed the base branch from release to feat/airgap-image-bundle April 5, 2023 09:22

berzerkeer requested a review from jsartisan April 5, 2023 09:23

berzerkeer self-assigned this Apr 5, 2023

github-actions bot added the Enhancement New feature or request label Apr 5, 2023

feat: reorder folders and remove old gitignore which ignored .svg in …

deb609c

…fonts folder

berzerkeer assigned jsartisan Apr 5, 2023

jsartisan added 2 commits April 6, 2023 10:32

remove fontloader

8565298

remove google font

efe7c48

jsartisan mentioned this pull request Apr 6, 2023

chore: Add font fallback to reduce FOUT #22061

Closed

13 tasks

Base automatically changed from feat/airgap-image-bundle to release April 10, 2023 07:02

Sangeeth Sivan added 2 commits April 10, 2023 12:59

Merge branch 'release' of github.com:appsmithorg/appsmith into feat/b…

90e0a10

…undle-fonts

Merge branch 'feat/bundle-fonts' of github.com:appsmithorg/appsmith i…

a03ca99

…nto feat/bundle-fonts

berzerkeer marked this pull request as ready for review April 10, 2023 07:36

github-actions bot added the Airgap Tickets related to supporting air-gapped Appsmith instances label Apr 10, 2023

fix: add fallback font in ci tests

15e51fe

jsartisan previously approved these changes Apr 12, 2023

View reviewed changes

app/client/src/pages/AppViewer/index.tsx Outdated Show resolved Hide resolved

ichik reviewed Apr 12, 2023

View reviewed changes

ichik previously approved these changes Apr 12, 2023

View reviewed changes

Sangeeth Sivan added 2 commits April 12, 2023 17:55

fix: theme form widget spec

0d05b4d

fix: chart spec

b216eea

berzerkeer dismissed stale reviews from ichik and jsartisan via b216eea April 12, 2023 12:26

fix: string quotes

6c346f2

jsartisan approved these changes Apr 12, 2023

View reviewed changes

ichik approved these changes Apr 12, 2023

View reviewed changes

trishaanand merged commit c574433 into release Apr 13, 2023

trishaanand deleted the feat/bundle-fonts branch April 13, 2023 13:18

dvj1988 mentioned this pull request Aug 5, 2024

chore: Remove google fonts imported from googleapis #35392

Merged

2 tasks

		@@ -0,0 +1,9 @@
		@import url("https://fonts.googleapis.com/css2?family=Nunito+Sans:ital,wght@0,300;0,400;0,700;1,300;1,400;1,700&display=swap");

Conversation

berzerkeer commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

22005

Type of change

How Has This Been Tested?

Checklist:

Dev activity

QA activity:

Uh oh!

jsartisan commented Apr 6, 2023

Uh oh!

github-actions bot commented Apr 6, 2023

Uh oh!

github-actions bot commented Apr 6, 2023

Uh oh!

github-actions bot commented Apr 6, 2023

Uh oh!

berzerkeer commented Apr 10, 2023

Uh oh!

github-actions bot commented Apr 10, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

berzerkeer commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

jsartisan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ichik Apr 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

berzerkeer commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

berzerkeer commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

berzerkeer commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

github-actions bot commented Apr 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

berzerkeer commented Apr 5, 2023 •

edited

Loading

ichik Apr 12, 2023 •

edited

Loading