Adding Posts analytics React app #21878
Conversation
peterzimon
force-pushed
the
posts-app
branch
2 times, most recently
from
ac50b8e to
452d374
Compare
peterzimon
force-pushed
the
posts-app
branch
from
45f08ac to
09d9e08
Compare
There was a problem hiding this comment.
Left a few comments - overall looks good but there are a lot of files with missing newlines at the end, which I think they should have - EditorConfig should be enforcing that for us.
Also the change to vite config in the admin-x-framework package, I'm not sure I fully understand that
| @@ -111,7 +111,8 @@ | |||
| "dependsOn": [ | |||
| "test:unit", | |||
| "^build", | |||
| "@tryghost/admin-x-design-system:test:unit" | |||
| "@tryghost/admin-x-design-system:test:unit", | |||
| "@tryghost/shade:test:unit" | |||
There was a problem hiding this comment.
This means that if the shade tests fail the admin-x-framework tests will fail too, is that what you want?
There was a problem hiding this comment.
Hm... Maybe not yet. We just added the current design system to it to catch DS bugs, but maybe we can leave Shade out until its first release. Good point.
| /> | ||
| </React.StrictMode> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Missing newline here - do you have the EditorConfig plugin installed in your editor?
There was a problem hiding this comment.
Good point, thx, fixed
| @@ -10,6 +11,11 @@ export default (function viteConfig() { | |||
| plugins: [ | |||
| react() | |||
| ], | |||
| resolve: { | |||
| alias: { | |||
| '@': path.resolve(__dirname, './src') | |||
There was a problem hiding this comment.
I don't see this alias used anywhere, can we revert the changes in this file?
| @@ -0,0 +1,60 @@ | |||
| import {cn} from '@/lib/utils'; | |||
There was a problem hiding this comment.
Oh I see the prefix is used here - this is a new pattern, am interested in discussing it further!
There was a problem hiding this comment.
So this is coming from ShadCN/UI. We could set it up differently but I quite like how clean this makes imports. We agreed to discuss this wider and maybe implement all across the codebase.
| @@ -67,13 +67,14 @@ | |||
| "@ebay/nice-modal-react": "1.2.13", | |||
| "@radix-ui/react-avatar": "1.1.0", | |||
| "@radix-ui/react-checkbox": "1.1.1", | |||
| "@radix-ui/react-dropdown-menu": "^2.1.3", | |||
There was a problem hiding this comment.
| "@radix-ui/react-dropdown-menu": "^2.1.3", | |
| "@radix-ui/react-dropdown-menu": "2.1.3", |
| "@radix-ui/react-form": "0.0.3", | ||
| "@radix-ui/react-popover": "1.1.1", | ||
| "@radix-ui/react-radio-group": "1.2.0", | ||
| "@radix-ui/react-separator": "1.1.0", | ||
| "@radix-ui/react-slot": "^1.1.0", | ||
| "@radix-ui/react-switch": "1.1.0", | ||
| "@radix-ui/react-tabs": "1.1.0", | ||
| "@radix-ui/react-tabs": "^1.1.2", |
There was a problem hiding this comment.
| "@radix-ui/react-tabs": "^1.1.2", | |
| "@radix-ui/react-tabs": "1.1.2", |
peterzimon
force-pushed
the
posts-app
branch
from
09d9e08 to
08a9b46
Compare
ref https://linear.app/ghost/issue/DES-1021/create-posts-app
Part of establishing React patterns in Ghost is to build a well-defined and fairly self-encapsulated app through which we can test assumptions and define best practices. Our guinea pig is Post analytics for this purpose. This PR creates a new React app (posts) using Shade (the new design system).