Skip to content

Initial Components: Device Details & Access Code Table #47

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

Merged
merged 58 commits into from
May 3, 2023

Conversation

mikewuu
Copy link
Contributor

@mikewuu mikewuu commented Apr 26, 2023

Moved lots, renamed lots. Not saying we have to keep the changes, but it was just easier to show most of it rather than typing out, so here it is. I suppose this PR is more of an RFC, it's meant to be taken apart, so we can hopefully land on some consistent conventions going forward.

Style guide mostly follows the Airbnb style guide.

Run npm run start to see the components in storybook

Screenshots

CleanShot 2023-04-26 at 13 59 38

Other Notable things

  • Removed /hooks/*. Personally I find it easier to find things when they're named by use-case (vs. type). Here being able to browse the seam api by directory, and seeing the resources available is helpful.
  • Turned off the explicit-return rule to let TS infer away.
  • We don't have webpack! Not sure if a module bundler is incoming, but in the meantime I've added a manual sass compile script, and used svg strings for images.
  • Don't have a watcher for scss file changes, so you'll need to re-run npm run build each time to have it take effect.
  • Not sure why TS is not happy with MUI components, could use a hand here:

TS Error for MUI components:

JSX element type 'Dialog' does not have any construct or call signatures.

@mikewuu mikewuu requested review from azat-co and razor-x as code owners April 26, 2023 05:00
@vercel
Copy link

vercel bot commented Apr 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
react ✅ Ready (Inspect) Visit Preview May 3, 2023 2:48pm

Copy link
Member

@razor-x razor-x left a comment

Choose a reason for hiding this comment

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

Getting late here, so please excuse any typos or terse sounding comments. Mostly just trying to go-fast b/c such PR, much big. This is a great start! Some of the structure changes are great, but have some concerns about others. It's important we sort those out now as it will get harder to change our mind later about some of these conventions.

@razor-x
Copy link
Member

razor-x commented Apr 26, 2023

Side note on the CSS stuff: even without a bundler for npm:build at the moment, we still need to make sure it can integrate into our dev flow for storybook / the examples which do have proper tooling.

@razor-x
Copy link
Member

razor-x commented Apr 26, 2023

I suppose this PR is more of an RFC, it's meant to be taken apart, so we can hopefully land on some consistent conventions going forward.

Missed this on my first look, but +1 to this approach!

Co-authored-by: Evan Sosenko <evan@getseam.com>
@mikewuu mikewuu force-pushed the access-code-table branch from fb8eaeb to cc3b456 Compare May 3, 2023 13:01
Copy link
Contributor

@seveibar seveibar left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Member

@razor-x razor-x left a comment

Choose a reason for hiding this comment

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

Agreed on early merge with @seveibar and will have a follow up PR instead of blocking review.

@seveibar seveibar merged commit 81c8eb0 into main May 3, 2023
@seveibar seveibar deleted the access-code-table branch May 3, 2023 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component Component development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants