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

Direct SVG rendering #12157

Merged
merged 1 commit into from
Jul 12, 2020
Merged

Direct SVG rendering #12157

merged 1 commit into from
Jul 12, 2020

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Jul 5, 2020 •
edited
Loading

This solves the issue of flashing SVG icons on page (re)load seen in some browsers (Firefox at least):

Introduce make svg which calls a node script that compiles svg files to public/img/svg which are then discovered by the backend during startup. These files are vendored to not create a dependency on Node for the backend build.

On the frontend side, configure webpack using raw-loader so SVGs can be imported as string. Also moved our existing SVGs to web_src/svg for consistency.

Fixes: #11618

@silverwind silverwind marked this pull request as draft July 5, 2020 21:31
@silverwind silverwind mentioned this pull request Jul 6, 2020
Closed
@silverwind
Copy link
Member Author

silverwind commented Jul 6, 2020 •
edited
Loading

I think I will use a small script ran on make svg to generate the svg icon build outside of webpack and then vendor the resulting files in public/img/svg. If we don't vendor, there would be a dependency on node/npm to build the backend which I want to avoid. Not involving webpack in the build has the benefit of having the single raw-loader for *.svg.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 6, 2020
@silverwind silverwind force-pushed the svgdirect branch 3 times, most recently from 248398b to 9337f48 Compare July 6, 2020 19:32
@silverwind silverwind changed the title WIP: Direct SVG rendering Direct SVG rendering Jul 6, 2020
@silverwind silverwind marked this pull request as ready for review July 6, 2020 19:36
@silverwind
Copy link
Member Author

Ready for review (assuming ci passes now).

@techknowlogick techknowlogick added type/refactoring Existing code has been cleaned up. There should be no new functionality. topic/ui Change the appearance of the Gitea UI labels Jul 7, 2020
@techknowlogick techknowlogick added this to the 1.13.0 milestone Jul 7, 2020
@silverwind
Copy link
Member Author

Added gif to OP.

lunny
lunny approved these changes Jul 8, 2020
View reviewed changes
Copy link
Member

@lunny lunny left a comment

Choose a reason for hiding this comment

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

Next PR we could allow custom svg icons.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 8, 2020
@lunny
Copy link
Member

lunny commented Jul 8, 2020

Please resolve the conflicts

@silverwind
Copy link
Member Author

silverwind commented Jul 8, 2020 •
edited
Loading

Next PR we could allow custom svg icons.

Our own custom SVG work, currently they are in assets/svg and will be in web_src/svg after this PR. What is not supported is SVGs in a user-supplied custom folder, but I'd say we don't need to support this as users can probably just paste their SVGs inline into the custom templates.

silverwind
silverwind commented Jul 9, 2020
View reviewed changes
package.json Outdated Show resolved Hide resolved
silverwind
silverwind commented Jul 9, 2020
View reviewed changes
modules/svg/svg.go Outdated Show resolved Hide resolved
@silverwind
Direct SVG rendering
dcbe085 
Introduce 'make svg' which calls a node script that compiles svg files
to `public/img/svg`. These files are vendored to not create a dependency
on Node for the backend build.

On the frontend side, configure webpack using `raw-loader` so SVGs can
be imported as string.

Also moved our existing SVGs to web_src/svg for consistency.

Fixes: go-gitea#11618
@silverwind
Copy link
Member Author

silverwind commented Jul 12, 2020 •
edited
Loading

Rebased and squashed

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jul 12, 2020
@lafriks lafriks merged commit 8188176 into go-gitea:master Jul 12, 2020
@silverwind silverwind deleted the svgdirect branch July 12, 2020 13:58
@CirnoT CirnoT mentioned this pull request Jul 12, 2020
Closed
7 tasks
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@lafriks lafriks lafriks approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.13.0
Development

Successfully merging this pull request may close these issues.

Directly render SVG paths
5 participants
@silverwind @lunny @lafriks @techknowlogick @GiteaBot