Move new tab page specific components and storybook from brave-ui #2450
Conversation
| @@ -0,0 +1,223 @@ | |||
| /* This Source Code Form is subject to the terms of the Mozilla Public | |||
There was a problem hiding this comment.
we should at some point standardize how we deal with locale both brave-core and brave-ui
There was a problem hiding this comment.
Yes that would be great. Just defining the strings once would be good. I thought perhaps webpack could take all the GRD strings that are already defined and output them in to a JSON file for storybook. However, it gets complicated when I realized that there are many different GRD files. Any suggestions?
There was a problem hiding this comment.
I like the idea. however, as you said, locale can be complicated as it also goes to third-party software for translators and I'm not sure how it works. but I guess what I'm looking for is a way to simplify adding new strings so non-dev folks could change the content. or at least finding a way to avoid writing for brave-ui and brave-core. we can elaborate it later in a future chat or PR
| @@ -12,6 +12,7 @@ module.exports = (env, argv) => ({ | |||
| alias: { | |||
| 'bittorrent-tracker': path.resolve(__dirname, '../../node_modules/bittorrent-tracker'), | |||
| 'brave-ui': path.resolve(__dirname, '../../node_modules/brave-ui/src'), | |||
| '~brave-core-ui': path.resolve(__dirname, '../../ui/webui/components'), | |||
There was a problem hiding this comment.
it seems to be the tilde is easy to miss and I'm afraid some people (me) would spend a good time figuring out what's wrong to realize it's missing. wdyt?
There was a problem hiding this comment.
That's a good point. I wanted to differentiate from @ prefix, which is valid prefix on npm. But we can probably just remove this if I can implement the overall 2nd proposal that we both wanted, instead of using ui/webui/components dir
There was a problem hiding this comment.
And of course this doesn't even need to be here for this architecture. It was just convenience as find / replace would be very time-consuming if it had to figure out the correct number of ../../ to put in to each replacement...
| @@ -265,18 +266,29 @@ | |||
| } | |||
| }, | |||
| "devDependencies": { | |||
| "@babel/core": "^7.4.4", | |||
There was a problem hiding this comment.
are there any cons of having a separate package.json for storybook? I'm thinking about people not familiar with brave-core wanting to add/remove deps in storybook and potentially removing something we need for other feature. I think it's unlikely that others would npm run build when working solely on storybook
There was a problem hiding this comment.
That's valid, but we'd have to weigh the benefits of that vs increase of developer overhead. How do you see it working? A storybook sub-project that the front-end developer must go to to run storybook. I wonder how that will work when webpack / ts in storybook sub-project it's building JS / TS output of features in parent project...
There was a problem hiding this comment.
I created a doc about things we like to consider including our first convo re locale strings. I'm ok with it in the same package.json until we better evaluate
There was a problem hiding this comment.
@cezaraugusto can we just use the existing doc?
|
closing after #2857 |
This is a 1st proposal (that will be replaced by a new idea.)
This PR moves:
brave-ui/src/features/newTab/... (and associated storybook stories) to:
brave-core/ui/webui/components/newTab/...
It renames all relevant JS imports from brave-core/components/brave_new_tab_ui/... from brave-ui to brave-core
It establishes
~brave-core-uias a fake module which resolves to /ui/webui/componentsIt installs storybook v5 with a similar configuration to brave-ui