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

#72 Storybook docs #78

Merged
merged 20 commits into from
Apr 2, 2024
Merged

#72 Storybook docs #78

merged 20 commits into from
Apr 2, 2024

Conversation

choir241
Copy link
Contributor

Created core categories to break down documentation into the following sections:

Welcome
About
Components
Documents

Added content to the Developer Guide to visualize basic layout and storybook default styling for documentations.

…or streamline, easy-to-navigate documentation for the application
…aking down into smaller sections for streamline, easy-to-navigate documentation for application. Add content to Developer Guide as a visualization for future documentation
Copy link

vercel bot commented Mar 19, 2024

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

Name Status Preview Comments Updated (UTC)
gridiron-survivor ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 2, 2024 5:10pm

@shashilo shashilo changed the title Storybook docs #67 Storybook docs Mar 22, 2024
@shashilo shashilo changed the title #67 Storybook docs Storybook docs Mar 22, 2024
@shashilo shashilo changed the title Storybook docs #72 Storybook docs Mar 22, 2024
Copy link
Collaborator

@shashilo shashilo left a comment

Choose a reason for hiding this comment

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

@choir27 A few comments for you to review. Can you capture a video of the changes you made? It's a lot easier to review than me spinning it up locally to review the visuals.

README.md Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to have this file in our documentation, it will serve as a good introduction page to the components, while also hosting a grid gallery of previews for how each component visually looks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But it's empty right now. Is it still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically right now it isn't needed as there is no content in it.

__tests__/Home.test.tsx Outdated Show resolved Hide resolved
components/Home.tsx Outdated Show resolved Hide resolved
package-lock.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
stories/About/Developer Guide.mdx Outdated Show resolved Hide resolved
@choir241
Copy link
Contributor Author

choir241 commented Apr 2, 2024

Copy link
Collaborator

@shashilo shashilo left a comment

Choose a reason for hiding this comment

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

@choir27 please review my comments.

<section>
<h3>Table of Contents</h3>
<ol>
<li><a href = "#started">Getting Started</a></li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be formated like this.

Suggested change
<li><a href = "#started">Getting Started</a></li>
<li><a href="#started">Getting Started</a></li>

<li><a href = "#query">Query Supabase data from Next.js</a></li>
</ol>

<h3 id = "started">Getting Started</h3>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here and thorought this PR code changes.. Fix the attribute formatting.

</div>


<style>
Copy link
Collaborator

Choose a reason for hiding this comment

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

@emestabillo These styles should live in its own SCSS file. Can you provide some direction or guidance on where this should live?

<Meta title="About/Developer Guide" />

<div className="sb-container">
<div className='sb-section-title'>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Inconsistent usage of double and single quotes. Work with @ryandotfurrer on the coding standards for this project.

<h3 id = "started">Getting Started</h3>
<ol>
<li>Clone the develop branch to your local machine:</li>
<li><code>git clone --single-branch --branch develop https://github.com/LetsGetTechnical/gridiron-survivor.git</code></li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is where the link should go to a new page. Anything outside of the Storybook infrastructure.

@choir241
Copy link
Contributor Author

choir241 commented Apr 2, 2024

Richard.mdx is neccessary for Documents section to be created in storybook
https://www.youtube.com/watch?v=IFutQJA-dNI&ab_channel=RichardChoi

Copy link
Collaborator

@shashilo shashilo left a comment

Choose a reason for hiding this comment

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

@choir27 Please review my comments.

@@ -4,8 +4,9 @@
"dev": "next dev",
"build": "next build",
"start": "next start",
"test": "jest",
"test:watch": "jest --watchAll",
"test": "jest --passWithNoTests",
Copy link
Collaborator

Choose a reason for hiding this comment

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

What was this param added to the script?

Copy link
Contributor Author

@choir241 choir241 Apr 2, 2024

Choose a reason for hiding this comment

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

When pulling from the develop's most recent changes (removing Home component and test folder), I was geting the following error in my PR when trying to push into the storybook-docs branch:

Run pnpm test

> @ test /home/runner/work/gridiron-survivor/gridiron-survivor
> jest

No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In /home/runner/work/gridiron-survivor/gridiron-survivor
  29 files checked.
  testMatch: **/__tests__/**/*.[jt]s?(x), **/?(*.)+(spec|test).[tj]s?(x) - 0 matches
  testPathIgnorePatterns: /node_modules/, /.next/ - 29 matches
  testRegex:  - 0 matches
Pattern:  - 0 matches
 ELIFECYCLE  Test failed. See above for more details.
Error: Process completed with exit code 1.

So after consulting with Mai to debug the error, after some research in regards to this particular error, she failed to duplicate the same error as my local machine/PR. Our inference is that my machine still remembers the tests, and therefore is still looking for them; therefore it fails the Docker test because there are no tests to look for.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this is ok for now until we add our 1st test. Then, we need to remove this.

@@ -16,38 +17,39 @@
"autoprefixer": "10.4.15",
"class-variance-authority": "^0.7.0",
"clsx": "^2.1.0",
"geist": "^1.0.0",
"geist": "^1.2.2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason why some of these packages were upgraded? We need to be very careful when upgrading versions.

Copy link
Contributor Author

@choir241 choir241 Apr 2, 2024

Choose a reason for hiding this comment

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

I don't recall upgrading anything when I was making changes, but I did have to pull from develop's new changes (removing Home component and test folder) in order to successfully make the push to my PR and merge into develop.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe this happened because there wasn't a package.lock file that was committed. So your installation command installed and updated to the latest packages. Once a package.lock file is in our repo, the only time we should upgrade our packages are when we need them to be upgraded. And not an automatic script that does it for us.

@choir241 choir241 merged commit ed2de99 into develop Apr 2, 2024
4 checks passed
@choir241 choir241 deleted the storybook-docs branch April 2, 2024 20:44
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.

Create Storybook documentation sections
4 participants