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.

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

Local fonts section implementation in React #267

Closed
wants to merge 1 commit into from

Conversation

matiasbenedetto
Copy link
Contributor

@matiasbenedetto matiasbenedetto commented Mar 9, 2023

What?

In this PR:

  • Implement the local font asset upload in the React App
  • Change the styles of this section to make it lool as the rest of the font section as implemented in Fonts outline sidebar #260

Why?

To allow us to implement the user ability to add more configuration options.

How to test:

  • Go to the "Add local font" page
  • Add a few fonts a test if everything is working as before.

Extra info:

⚠️ This PR has as base the branch of #260 to re-use some of the components implemented in that branch.
Before merging it we need to use trunk as base.

@madhusudhand
Copy link
Member

Can you point the PR to trunk?

@matiasbenedetto
Copy link
Contributor Author

Can you point the PR to trunk?

If I do that all the changes from add/fonts-sidebar will show in the diff. What would you want to do that?

@madhusudhand
Copy link
Member

If I do that all the changes from add/fonts-sidebar will show in the diff. What would you want to do that?

No. It adds more noise.

I would like this to be independent with related changes so it would be easy to review and merge.
If you think it has to depend on the changes from #260, let's mark this as draft PR for time being.

@matiasbenedetto
Copy link
Contributor Author

I'm closing this PR in favor of this other one that can be merged independently of other code changes: #268

@madhusudhand madhusudhand deleted the refactor/local-fonts branch March 16, 2023 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants