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

Tomb component docs #132

Open
wants to merge 7 commits into
base: next
Choose a base branch
from
Open

Tomb component docs #132

wants to merge 7 commits into from

Conversation

xsanctom
Copy link

@xsanctom xsanctom commented Sep 30, 2022

docs: added amazing component documentation templates

πŸ“¦ Published PR as canary version: Canary Versions

✨ Test out this PR locally via:

npm install @tablecheck/[email protected]
npm install @tablecheck/[email protected]
npm install @tablecheck/[email protected]
# or 
yarn add @tablecheck/[email protected]
yarn add @tablecheck/[email protected]
yarn add @tablecheck/[email protected]

@github-actions
Copy link

github-actions bot commented Sep 30, 2022

Unit Test Results

0 tests  Β±0   0 βœ”οΈ Β±0   0s ⏱️ Β±0s
0 suites Β±0   0 πŸ’€ Β±0 
1 files   Β±0   0 ❌ Β±0 

Results for commit 06a928e. ± Comparison against base commit d191838.

♻️ This comment has been updated with latest results.

@SimeonC SimeonC changed the base branch from main to next September 30, 2022 10:19
@SimeonC
Copy link
Contributor

SimeonC commented Oct 3, 2022

I think I prefer us to use more real code examples rather than images, or embedded SVGs for the case where we want some measurements.

One of the reasons for this is that we can have them adapt to dark/light mode and the other is that we have a lot of actual code examples rather than view only images.

@gazpachu gazpachu added the patch Increment the patch version when merged label Oct 4, 2022
@gazpachu
Copy link
Contributor

gazpachu commented Oct 4, 2022

I agree with Simeon's opinion, but I think we can merge this for now, so we can have a base to work on those improvements.

Also, I'd prefer if we move the Components to the Dev section. @xsanctom if you want I can do that and also squash the commits. ok?

@SimeonC
Copy link
Contributor

SimeonC commented Oct 4, 2022

I think once we've squashed the commits/pushed a new commit we should get the preview deploy to review. Originally this was targeting main not next which is why I think it didn't trigger.

But yea, dealing with the component previews in another PR is OK. Though I would like to remove the images for those for now and add them in other PRs when necessary as images generally shouldn't be committed to a repo if possible. The way git works is that every image ever committed into all branches are downloaded when you check out the repo so I want to avoid that if possible.

@gazpachu
Copy link
Contributor

gazpachu commented Oct 7, 2022

@SimeonC where do you suggest uploading the images (the ones with the measurements)?

@SimeonC
Copy link
Contributor

SimeonC commented Oct 7, 2022

TBH I've not got any good ideas around that. I've seen all sorts of things from using imgur or other free hosting sites, uploading into a github comment then using the CDN link that it resolves to. Maybe @DragonStuff has some good ideas? I kinda want that image CMS we talked about ages ago, just something that would allow us to have a S3 bucket/cdn-ish for these kind of images. (both for TK and also for our other projects like website/diner app etc)

@netlify
Copy link

netlify bot commented Oct 10, 2022

βœ… Deploy Preview for tablekit ready!

Name Link
πŸ”¨ Latest commit 06a928e
πŸ” Latest deploy log https://app.netlify.com/sites/tablekit/deploys/635746e51f4b3d000a0d7566
😎 Deploy Preview https://deploy-preview-132--tablekit.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 settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants