-
Notifications
You must be signed in to change notification settings - Fork 22
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
Adding new scaffold to the react folder #16
Conversation
packages/react/README.md
Outdated
@@ -1,205 +1,152 @@ | |||
*Use of this software is subject to important terms and conditions as set forth in the License file* | |||
# ReactJS Scaffold Proposal |
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.
# ReactJS Scaffold Proposal | |
# ReactJS Scaffod |
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.
Actually - I think we probably need to re-write the README. It mostly reads as a proposal rather than a project README. Could you revert to the old version and update anything that's changed?
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.
Sure i will work in this change
packages/react/package.json
Outdated
"build": "vite build --mode production", | ||
"lint": "eslint . --ext js,jsx --report-unused-disable-directives --max-warnings 0", | ||
"format": "prettier --write .", | ||
"test": "vitest" |
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.
Mind if we match the previous commands? I suspect there's some documentation for it in various places. Probably easier to just keep it the same if possible.
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.
I will review the preview ones and match them to be the same, just keep in mind we have two stages one for development and one for production (Once we are ready to submit the production version), we might have one extra comand.
"react-dom": "^18.2.0" | ||
}, | ||
"devDependencies": { | ||
"@testing-library/dom": "^10.1.0", |
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.
I think including Zendesk garden by default is a good way to encourage developers to use it, and would (hopefully) lead to more apps that match Zendesk's UI. Any reason we pulled this out?
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.
No reason at all, i will add the library as the previous one
packages/react/src/app/index.css
Outdated
--color-primary-dark: #357ebd; /* Tailwind's blue-600 */ | ||
--color-primary-darker: #276bb0; /* Tailwind's blue-700 */ | ||
--color-white: #ffffff; /* Tailwind's white */ | ||
--color-gray-100: #f7fafc; /* Tailwind's gray-100 */ |
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.
Similar comment to https://github.com/zendesk/app_scaffolds/pull/16/files#r1650215828. Rather than using tailwing colors, any reason we can't pull in Zendesk garden colors?
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.
I forgot to change this, but once we add Zendesk garden library we might remove this part
packages/react/src/manifest.json
Outdated
"name": "John Doe", | ||
"email": "support@zendesk.com", | ||
"name": "Daniel Montes", | ||
"email": "daniela.montes@zendesk.com", |
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.
Can we revert this (unless you want a bunch of direct emails 😆)?
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.
Yep i will change this too.
"url": "https://www.zendesk.com" | ||
}, | ||
"defaultLocale": "en", | ||
"private": true, | ||
"location": { | ||
"support": { | ||
"ticket_sidebar": { | ||
"url": "assets/iframe.html", | ||
"url": "http://localhost:3000/", |
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.
I haven't tested this, but it looks like this wouldn't work in production - how are we handling this?
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.
So the main purpose for this is to have a better developing experience so the app has two stages one for development and the other for production, this value will be change automatically once we build the production version. (This is mention in the README, one of the big changes here from the previous version is that in this one we have two stages one for development and one for production, hope this still match the goal of the react scaffold )
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.
Got it. I think that's fine. I'll have a play around with it to see if it causes any unexpected issues.
@JeanMarcGoepfert Change have been made, let me know if there is something else we might need to change/add or remove. Things to highlight when adding Zendesk Garden with the most recent version of react:
When installing the latest version of styled-component most of the props are not being handle correctly as it is showing this type of errors:
This is might not be something to change on the zendesk garden side maybe styled-components made some changes i just use the previous version we are using v5. |
Man, you are my hero 🔥🔥🔥 |
@zendesk/vegemite
Description
We're introducing a new React scaffold designed to optimize Zendesk app development. Leveraging Vite for expedited builds and robust hot module replacement (HMR), we emphasize modern React hooks for streamlined development and simplified configuration and asset management.
Key Features:
Accelerated builds and advanced HMR capabilities using Vite
Modern React patterns with a focus on hooks for enhanced developer efficiency
Simplified configuration and efficient static asset handling
The scaffold supports two environments: one for development with instant feedback through HMR, and another for production, ready for packaging and deployment.
Your feedback is crucial as we refine this solution for seamless integration into Zendesk's ecosystem.
@JeanMarcGoepfert