-
Notifications
You must be signed in to change notification settings - Fork 0
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
CFWD Feedback / Updates #151
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Some really nice update.
One thought with the minifying theme.json if you have a different view, let me know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Glad you updated this as well!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dang this is cool! Did you build this from scratch or find a demo?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly Stack Overflow 🤣
@@ -1,4 +1,4 @@ | |||
//Move any block that you are working to a new file to make it easyer to maintain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😅 good catch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I was minifying this is because it saves some file size but more importantly it made it hard to edit the file directly, as any edit should be done in the JS file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we just need to make sure that's clear from the README/Documentation side, however, I think there are a few good reasons to have it formatted:
- Once the project is handed off, the client may not have the ability to use the build tools, or understand how to manage this file with the provided JS parts.
- Debugging can be difficult, and this makes it easier to determine if styles are coming from
theme.json
or somewhere else (in a CSS file). It can also be useful to confirm settings are stored as they are expected to be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point with the client having the option to edit it post hand off. I like it!
Summary
This PR incorporates some of the discoveries found in the CFWD Project. In summary:
theme.json
filesrc
folder.Issues
Testing Instructions
N/A
Screenshots
N/A