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

Fira fonts require GitHub to install #1902

Closed
mofojed opened this issue Mar 25, 2024 · 2 comments · Fixed by #1944 or deephaven/deephaven-core#5439
Closed

Fira fonts require GitHub to install #1902

mofojed opened this issue Mar 25, 2024 · 2 comments · Fixed by #1944 or deephaven/deephaven-core#5439
Assignees
@wusteven815
Labels
bug Something isn't working
Milestone
April 2024

Comments

@mofojed
Copy link
Member

mofojed commented Mar 25, 2024

All of our dependencies are stored in the npmjs registry, except for fira, which references the GitHub URL directly:

web-client-ui/packages/app-utils/package.json

Line 51 in b49b506

"fira": "mozilla/fira#4.202"

That could be a problem when installing the package, as a) They may have access to pulling from GitHub blocked/locked down, and/or b) That repository is not guaranteed to always exist; the packages published on npmjs should always be there.
@mofojed mofojed added bug Something isn't working triage Issue requires triage labels Mar 25, 2024
@dsmmcken
Copy link
Contributor

I forget the reason we did this, but there was one.

@dsmmcken
Copy link
Contributor

In enterprise we use to embed them in the repo directly. I think we switched to a package to get license provenance.

(non-accessible commit switching it in core repo before it became deephaven-core, while it was still being split)
https://github.com/deephaven/core/pull/344/files

At the time there was no versions of it on npm. I looked at @fontsource/fira-sans and that would probably be fine. I can confirm it is correctly publishing the font with feature settings for tabular numbers preserved.

@vbabich vbabich added this to the April 2024 milestone Mar 26, 2024
@vbabich vbabich removed the triage Issue requires triage label Mar 26, 2024
@wusteven815 wusteven815 mentioned this issue Apr 23, 2024
Merged
@wusteven815 wusteven815 linked a pull request Apr 23, 2024 that will close this issue
fix: change fira source #1944
Merged
wusteven815 added a commit that referenced this issue Apr 26, 2024
@wusteven815 @dsmmcken
fix: change fira source (#1944)
07e5a26 
- Adds #1902
- Use `@fontsource` instead of GitHub as source
- Lock to Fira Sans to 5.0.20 and Fira Mono to 5.0.13 to prevent kerning
problems

---------

Co-authored-by: Don <dsmmcken@gmail.com>
@mofojed mofojed mentioned this issue May 1, 2024
Merged
mofojed pushed a commit to deephaven/deephaven-core that referenced this issue May 1, 2024
@github-actions @deephaven-internal
Update web version 0.75.0 (#5439)
3a34e5c 
Release notes https://github.com/deephaven/web-client-ui/releases/tag/v0.75.0

# [0.75.0](deephaven/web-client-ui@v0.74.0...v0.75.0) (2024-05-01)


### Bug Fixes

* change fira source ([#1944](deephaven/web-client-ui#1944)) ([07e5a26](deephaven/web-client-ui@07e5a26)), closes [#1902](deephaven/web-client-ui#1902)
* Fix null partition filter ([#1954](deephaven/web-client-ui#1954)) ([3a1f92b](deephaven/web-client-ui@3a1f92b)), closes [#1867](deephaven/web-client-ui#1867)


### Features

* context menu reopen for stack only ([#1932](deephaven/web-client-ui#1932)) ([6a9a6a4](deephaven/web-client-ui@6a9a6a4)), closes [#1931](deephaven/web-client-ui#1931)
* Create an ErrorView that can be used to display errors ([#1965](deephaven/web-client-ui#1965)) ([65ef1a7](deephaven/web-client-ui@65ef1a7))
* ListView + Picker - Item icon support ([#1959](deephaven/web-client-ui#1959)) ([cb13c60](deephaven/web-client-ui@cb13c60)), closes [#1890](deephaven/web-client-ui#1890)
* Picker - initial scroll position ([#1942](deephaven/web-client-ui#1942)) ([5f49761](deephaven/web-client-ui@5f49761)), closes [#1890](deephaven/web-client-ui#1890) [#1935](deephaven/web-client-ui#1935)

Co-authored-by: deephaven-internal <66694643+deephaven-internal@users.noreply.github.com>
stanbrub pushed a commit to deephaven/deephaven-core that referenced this issue May 3, 2024
@github-actions @deephaven-internal @stanbrub
Update web version 0.75.0 (#5439)
41a93da 
Release notes https://github.com/deephaven/web-client-ui/releases/tag/v0.75.0

# [0.75.0](deephaven/web-client-ui@v0.74.0...v0.75.0) (2024-05-01)


### Bug Fixes

* change fira source ([#1944](deephaven/web-client-ui#1944)) ([07e5a26](deephaven/web-client-ui@07e5a26)), closes [#1902](deephaven/web-client-ui#1902)
* Fix null partition filter ([#1954](deephaven/web-client-ui#1954)) ([3a1f92b](deephaven/web-client-ui@3a1f92b)), closes [#1867](deephaven/web-client-ui#1867)


### Features

* context menu reopen for stack only ([#1932](deephaven/web-client-ui#1932)) ([6a9a6a4](deephaven/web-client-ui@6a9a6a4)), closes [#1931](deephaven/web-client-ui#1931)
* Create an ErrorView that can be used to display errors ([#1965](deephaven/web-client-ui#1965)) ([65ef1a7](deephaven/web-client-ui@65ef1a7))
* ListView + Picker - Item icon support ([#1959](deephaven/web-client-ui#1959)) ([cb13c60](deephaven/web-client-ui@cb13c60)), closes [#1890](deephaven/web-client-ui#1890)
* Picker - initial scroll position ([#1942](deephaven/web-client-ui#1942)) ([5f49761](deephaven/web-client-ui@5f49761)), closes [#1890](deephaven/web-client-ui#1890) [#1935](deephaven/web-client-ui#1935)

Co-authored-by: deephaven-internal <66694643+deephaven-internal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wusteven815 wusteven815

Labels
bug Something isn't working
Projects
None yet
Milestone
April 2024
4 participants
@dsmmcken @vbabich @mofojed @wusteven815