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

[Design System] - New Package #5467

Merged
merged 12 commits into from May 17, 2024
Merged

[Design System] - New Package #5467

merged 12 commits into from May 17, 2024

Conversation

antonjoel82
Copy link
Contributor

@antonjoel82 antonjoel82 commented Apr 29, 2024

What changed? Why was the change needed?

  • Create a new package for the design system!
  • Bring-in Panda tokens + setup
  • Update build systems such that we can use a single import for our components & Panda-generated code
  • The name novui (Novu + UI) is not final and can be replaced. It was used to be clearly different than the existing package so as not to confuse the two

Solves NV-3691 and NV-3693

Notes

Screenshots

This incredible UI showcases a component built in the new DS package next to a component with styles overwritten in web using the generated panda code imported from the new package -- all using tokens for colors

Screenshot 2024-05-13 at 12 03 13 PM

Future iterations

  • Turn-on "strict" tokens for web -- currently this causes a lot of errors for tokens that shouldn't seemingly be excluded from strict

Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for dev-web-novu ready!

Name Link
🔨 Latest commit 686c09b
🔍 Latest deploy log https://app.netlify.com/sites/dev-web-novu/deploys/6647b6e00da9670008ff292e
😎 Deploy Preview https://deploy-preview-5467--dev-web-novu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 29, 2024

Deploy Preview for novu-design ready!

Name Link
🔨 Latest commit 686c09b
🔍 Latest deploy log https://app.netlify.com/sites/novu-design/deploys/6647b6e01adc3400088b4978
😎 Deploy Preview https://deploy-preview-5467--novu-design.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@antonjoel82 antonjoel82 changed the title Ds new pkg [Design System] - New Package May 1, 2024
@antonjoel82 antonjoel82 force-pushed the ds-new-pkg branch 3 times, most recently from f3776e9 to 714bde9 Compare May 8, 2024 01:56
@@ -10,7 +10,7 @@
"forceConsistentCasingInFileNames": true,
"noFallthroughCasesInSwitch": true,
"module": "esnext",
"moduleResolution": "node",
"moduleResolution": "Node16",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🗒 note (non-blocking): This was necessary to enable using the "new" exports field in package.json (in novui/package.json). This permits imports like @novu/novui/css which actually points to ./styled-system/css instead of dist

@@ -1,7 +1,7 @@
import { MembersTable } from './MembersTable';
import { TestWrapper } from '../../../testing';
import { MemberRoleEnum, MemberStatusEnum } from '@novu/shared';
import { SinonSpy } from 'cypress/types/sinon';
import { SinonSpy } from 'sinon';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🗒 note (non-blocking): This broke when changing module resolution in tsconfig -- also just not ideal anyway‏

@@ -1,7 +1,7 @@
import { FC, PropsWithChildren, useState } from 'react';
import { NavLink } from 'react-router-dom';
import { css } from '../../../styled-system/css';
import { HStack } from '../../../styled-system/jsx';
import { css } from '@novu/novui/css';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🗒 note (non-blocking): ‏Example imports using the new DS

Comment on lines +10 to +45
".": {
"source": "./src/index.ts",
"types": "./dist/index.d.ts",
"import": "./dist/index.js"
},
"./css": {
"types": "./styled-system/css/index.d.ts",
"require": "./styled-system/css/index.js",
"import": "./styled-system/css/index.js"
},
"./tokens": {
"types": "./styled-system/tokens/index.d.ts",
"require": "./styled-system/tokens/index.js",
"import": "./styled-system/tokens/index.js"
},
"./types": {
"types": "./styled-system/types/index.d.ts",
"require": "./styled-system/types/index.js",
"import": "./styled-system/types/index.js"
},
"./patterns": {
"types": "./styled-system/patterns/index.d.ts",
"require": "./styled-system/patterns/index.js",
"import": "./styled-system/patterns/index.js"
},
"./recipes": {
"types": "./styled-system/recipes/index.d.ts",
"require": "./styled-system/recipes/index.js",
"import": "./styled-system/recipes/index.js"
},
"./jsx": {
"types": "./styled-system/jsx/index.d.ts",
"require": "./styled-system/jsx/index.js",
"import": "./styled-system/jsx/index.js"
},
"./styles.css": "./styled-system/styles.css"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🗒 note (non-blocking): Enables better imports for package consumers. Requires moduleResolution: >= Node16 in the consumer‏

Comment on lines 13 to 14
import { Test } from '@novu/novui';
import { css } from '@novu/novui/css';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🗒 note (non-blocking): ‏Example of new imports!

@antonjoel82 antonjoel82 marked this pull request as ready for review May 14, 2024 23:44
@antonjoel82 antonjoel82 requested review from a team as code owners May 14, 2024 23:44
@antonjoel82 antonjoel82 requested review from scopsy and AliaksandrRyzhou and removed request for a team May 14, 2024 23:44
<NoDataSubHeading className={css({ color: 'legacy.success !important' })}>
Start from a blank workflow or use a template
</NoDataSubHeading>
<Test />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

🗒 note (non-blocking): We will remove this before merging! This is just to validate that everything is working across the two‏

@antonjoel82 antonjoel82 merged commit d887144 into next May 17, 2024
12 of 14 checks passed
@antonjoel82 antonjoel82 deleted the ds-new-pkg branch May 17, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants