Skip to content
This repository has been archived by the owner on Oct 17, 2020. It is now read-only.

Add story for Table component #1001

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add story for Table component #1001

wants to merge 4 commits into from

Conversation

Coteh
Copy link
Collaborator

@Coteh Coteh commented Aug 22, 2020

Addresses the Table story in #954.

Leaving in Draft until these items are done:

  • Complete knobs for columns widths and header font size
  • Determine whether it's necessary to have knobs for align headers and align body columns
  • Decide whether to transition from knobs to Storybook Controls edit: will address in separate PR

This PR also adds Storybook Backgrounds, as I wanted to add the color #fafafa, which is being used underneath the Favourite short links table in the homepage, as a background colour for the canvas. This will make the Table component stand out better. I also set #fafafa as the default background colour for story canvases.

image

@codecov
Copy link

codecov bot commented Aug 22, 2020

Codecov Report

Merging #1001 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1001   +/-   ##
=======================================
  Coverage   52.58%   52.58%           
=======================================
  Files         142      142           
  Lines        3564     3564           
  Branches      168      168           
=======================================
  Hits         1874     1874           
  Misses       1623     1623           
  Partials       67       67           
Flag Coverage Δ
#golang 72.03% <ø> (ø)
#typescript 22.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
frontend/src/component/ui/Table.tsx 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ebc6a04...c243c26. Read the comment docs.

@Coteh Coteh marked this pull request as ready for review August 22, 2020 20:28
@Coteh
Copy link
Collaborator Author

Coteh commented Aug 22, 2020

Readied this PR for review, will convert it to Storybook Controls if we start using it.

@magicoder10
Copy link
Member

@Coteh Control seems good. Let's do it!

@Coteh
Copy link
Collaborator Author

Coteh commented Aug 28, 2020

@byliuyang I think what I'll do is merge this in as-is, then make a separate PR for Controls and convert all existing stories to use Controls in that PR. What do you think?

Copy link
Member

@magicoder10 magicoder10 left a comment

Choose a reason for hiding this comment

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

Hi @Coteh , thank you so much for working on this PR. Can you add a screenshot so that reviewers know how does the page look like?

Comment on lines +31 to +33
rows={new Array(number('Number of rows', 2))
.fill(0)
.map((_, i) => headers.map((_, j) => `data ${i}-${j}`))}
Copy link
Member

Choose a reason for hiding this comment

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

I am confused by what's going on here. Can you explain what's going on or extract the logic using helper functions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am simply allowing the table to have the number of rows user specifies within the storybook. I can make it into a helper function if that makes it easier to understand.

Comment on lines +1 to +15
export const parameters = {
backgrounds: {
default: 'short',
values: [
{
name: 'white',
value: '#fff'
},
{
name: 'short',
value: '#fafafa'
}
]
}
};
Copy link
Member

Choose a reason for hiding this comment

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

I wonder what are those?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are the parameters for storybook backgrounds https://storybook.js.org/docs/react/essentials/backgrounds

@Coteh
Copy link
Collaborator Author

Coteh commented Sep 2, 2020

@byliuyang Screenshot added

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants