Skip to content

Resolve #2728: Added fontsource usage instead google fonts #2731

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
Closed
wants to merge 5 commits into from
Closed

Resolve #2728: Added fontsource usage instead google fonts #2731

wants to merge 5 commits into from

Conversation

0leg53
Copy link

@0leg53 0leg53 commented Oct 12, 2022 •
edited by David-DB88
Loading

  • Breaking change? (if so, please describe the impact and migration path for existing application instances)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)

  • No need to
  • Manually (please, describe, if necessary)
  • Unit checks
  • Integration checks
  • Covered by existing automation

Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)

  • 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 (e.g. ENVIRONMENT VARIABLES)
  • My changes generate no new warnings (e.g. Sonar is happy)
  • 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
  • Any dependent changes have been merged

Check out Contributing and Code of Conduct

Linked issues:
Closes #2728

@0leg53 0leg53 requested a review from a team as a code owner October 12, 2022 09:06
@0leg53 0leg53 changed the title Resolve 2728 Resolve 2728: Added fontsource usage instead google fonts Oct 12, 2022
@0leg53 0leg53 changed the title Resolve 2728: Added fontsource usage instead google fonts Resolve #2728: Added fontsource usage instead google fonts Oct 12, 2022
github-actions[bot]
github-actions bot reviewed Oct 12, 2022
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello there 0leg53! 👋

Thank you and congrats 🎉 for opening your first PR on this project! ✨ 💖

We will try to review it soon!

@workshur workshur linked an issue Oct 13, 2022 that may be closed by this pull request
Implement a full support for custom context path #2372
Closed
@workshur
Copy link
Member

Hey @0leg53,
Thank you for contribution!
Unfortunately, we cannot merge your changes yet. Due to the technical peculiarities of building and running the application.

This PR is blocked by #2372

@workshur workshur added status/pending Further information is requested scope/frontend status/blocked Further development process is blocked by something. Prevents merging for PRs. labels Oct 13, 2022
@Haarolean Haarolean added the type/enhancement En enhancement to an already existing feature label Oct 13, 2022
@github-actions
Copy link

This PR has been automatically marked as stale because no requested changes have been applied. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the status/stale Stale issues will be closed in 7 days. label Oct 21, 2022
@Haarolean Haarolean removed status/pending Further information is requested status/stale Stale issues will be closed in 7 days. labels Oct 21, 2022
@Haarolean Haarolean self-requested a review October 21, 2022 21:51
@Haarolean Haarolean self-requested a review October 24, 2022 11:53
Haarolean
Haarolean suggested changes Oct 24, 2022
View reviewed changes
Copy link
Contributor

@Haarolean Haarolean left a comment

Choose a reason for hiding this comment

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

image
yep, doesn't work good with custom context path

@Haarolean Haarolean marked this pull request as draft October 25, 2022 19:54
@Haarolean
Copy link
Contributor

@0leg53 I've added a label "hacktoberfest-accepted" to make sure it does count towards the goal if you're participating

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@0leg53
Copy link
Author

0leg53 commented Oct 26, 2022

@Haarolean
thank you for review.
how exactly you've got this error? I've tried to run app with pnpm preview and font loading looks fine.
image

@Haarolean
Copy link
Contributor

@0leg53 you'll have to set a SERVER_SERVLET_CONTEXT_PATH property. This is a result of #2372 as Oleg mentioned :)

@Haarolean
Copy link
Contributor

@0leg53 hey can you fix the conflicts? I think we're ready to test it out.

@Haarolean Haarolean added status/pending Further information is requested and removed status/blocked Further development process is blocked by something. Prevents merging for PRs. labels Dec 27, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Mgrdich
Mgrdich reviewed Dec 28, 2022
View reviewed changes
Copy link
Contributor

@Mgrdich Mgrdich left a comment

Choose a reason for hiding this comment

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

a Question in general , is it a good behavior to actually include the font in the javascript bundled file, what if we actually put it in a separate css file with font faces and everything , and include that css in the HTML itself.

@github-actions
Copy link

github-actions bot commented Jan 5, 2023

This PR has been automatically marked as stale because no requested changes have been applied. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the status/stale Stale issues will be closed in 7 days. label Jan 5, 2023
@github-actions github-actions bot closed this Jan 12, 2023
@Haarolean Haarolean reopened this Feb 15, 2023
@Haarolean Haarolean removed their assignment Feb 15, 2023
@Haarolean Haarolean removed status/pending Further information is requested status/stale Stale issues will be closed in 7 days. hacktoberfest-accepted labels Feb 15, 2023
@Haarolean Haarolean linked an issue Feb 15, 2023 that may be closed by this pull request
Kafka UI doesn't work without internet connection #3248
Open
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@David-DB88
Merge branch 'master' into issues/2728
9100aca 
# Conflicts:
#	kafka-ui-react-app/index.html
#	kafka-ui-react-app/package.json
#	kafka-ui-react-app/pnpm-lock.yaml
@David-DB88
resolve conflicts
00d5c4b
@David-DB88
pnpm-lock.yaml is not up-to-date with package.json
d642608
@David-DB88
jest 29.5.0 install version
e57319d
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Haarolean Haarolean assigned Mgrdich and unassigned David-DB88 Mar 20, 2023
@Haarolean Haarolean assigned NeiruBugz and unassigned Mgrdich Mar 30, 2023
@Haarolean
Copy link
Contributor

Done within #3602

@Haarolean Haarolean closed this Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@Mgrdich Mgrdich Mgrdich left review comments

@Haarolean Haarolean Haarolean requested changes

Assignees

@NeiruBugz NeiruBugz

Labels
scope/frontend type/enhancement En enhancement to an already existing feature
Projects
None yet
Milestone
No milestone
6 participants
@0leg53 @workshur @Haarolean @Mgrdich @NeiruBugz @David-DB88