Skip to content

Conversation

@scotthallock
Copy link
Contributor

Overview

This PR adds storybook to DockerPulse (in the /ui folder).

The npx storybook@latest init command was used to install storybook: https://storybook.js.org/docs/react/get-started/install

To run storybook:

cd ui
npm run storybook

This will compile and serve a development build of the Storybook that reflects the source code changes in the browser in real time.

Example story files were deleted from the installation, and I created our first story for the NavBar component.

Screenshot 2023-06-26 at 4 27 15 PM

@patrickv77
Copy link
Contributor

Approved but just a small observation:

Across this PR, the export statements are uniform, but they are different from the format we've been using across the project. I'll leave it up to you to decide if it is worth it to maintain uniformity in regards to the export statements.

@scotthallock
Copy link
Contributor Author

Across this PR, the export statements are uniform, but they are different from the format we've been using across the project. I'll leave it up to you to decide if it is worth it to maintain uniformity in regards to the export statements.

Good point, and I did look into this which led me down a little TS rabbit hole. Here's what I learned:

export default someFunction() { ... } is what we are using on other files (especially the react components), but in this PR we are primarily exporting objects, not functions.

Here's a really good discussion on this exact topic: microsoft/TypeScript#13626

You can't default export a named object:
Screenshot 2023-06-27 at 9 30 52 AM

While this syntax is valid, it is type-casting the object and therefore you purposefully lose type safety:
Screenshot 2023-06-27 at 9 33 00 AM

@scotthallock scotthallock merged commit 9195d08 into dev Jun 27, 2023
@scotthallock scotthallock deleted the storybook branch June 30, 2023 01:54
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.

4 participants