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

add shared-components package #449

Merged
merged 3 commits into from
Sep 14, 2022
Merged

add shared-components package #449

merged 3 commits into from
Sep 14, 2022

Conversation

mannynotfound
Copy link
Contributor

@mannynotfound mannynotfound commented Sep 13, 2022

Adds frontend/packages/shared-components as the place where components that are re-used many times to live. Currently this includes the following components, listed in order of the amount of files they appear in:

Svg - 31
Loader - 13
FadeIn - 13
WrapperResponsive - 12
ActionButton - 11
WalletConnect - 6
Input - 5
Dropdown - 5
Error - 5
Label - 3
AddButton - 3
Tooltip - 2
Title - 2
Tablink - 2
StatusLabel - 2

This PR only handles the Svg component, turning into a high order component that takes a name prop and lazy loads the corresponding Svg icon, based on the work of Linxiao in DapperCollectives/VESSEL#102

Additionally, this PR adds storybook to the shared-components repo so we can easily test, document, and iterate on shared components there. In follow up PRs I will move the above components to shared-components, add global styles and any necessary storybook addons.

image

@linear
Copy link

linear bot commented Sep 13, 2022

CAS-481

@@ -10,10 +10,10 @@
"workspaces": {
"packages": [
"packages/*"
],
"nohoist": [
"**"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nohoist: ** was removed since we need packages to share the same node_modules for things like react and react-dom

"dependencies": {},
"resolutions": {
"babel-loader": "8.1.0"
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this babel-loader resolution was added to resolve the following problem: storybookjs/storybook#5183

return webpackConfig;
},
},
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

craco is used so we can load things outside of packages/client with our create-react-app babel config

@@ -1,7 +1,10 @@
{
"compilerOptions": {
"baseUrl": "src"
"baseUrl": "src",
"jsx": "react-jsx"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add jsx: react-jsx so we dont have to import React from 'react' everywhere just to use JSX

"@tanstack/react-query": "^4.1.0",
"@tanstack/react-query-devtools": "4.0.11-beta.0",
"@tanstack/react-query": "4.3.3",
"@tanstack/react-query-devtools": "4.3.3",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to specify exact versions because I was running into this bug that was seemingly introduced yesterday: TanStack/query#4155

'Twitter',
'Upload',
'ValidCheckMark',
'Website',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately have to maintain a list of options for the storybook as there wasnt a pretty way to dynamically get all the filenames of the Svgs the way we do with dynamic import

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a way to show all icons in one component, so we can easily view them/search for them

Copy link
Contributor

@germanurrus germanurrus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good! Tested locally and works fine 👍

@@ -1,7 +1,10 @@
{
"compilerOptions": {
"baseUrl": "src"
"baseUrl": "src",
"jsx": "react-jsx"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are still some import React from 'react': in the components code, but can safely be removed later on a second PR to avoid increasing the size if this one (It's not the purpose of the PR)
Screen Shot 2022-09-14 at 11 35 03

'Twitter',
'Upload',
'ValidCheckMark',
'Website',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a way to show all icons in one component, so we can easily view them/search for them

export const Controls = Template.bind({});
Controls.args = {
name: 'Eye',
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have this file inside the Svg folder? So each shared component has it's own .stories file and on a high level we have folders only. Also for updating the .stories file and at the same time the component structure, might be easier and closer

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