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

feat(producttable): add isvisible/hidden icons #5407

Merged
merged 3 commits into from
Aug 2, 2019

Conversation

machikoyasuda
Copy link
Contributor

Resolves #5392
Impact: minor
Type: feature

Issue

#5392

Solution

Screen Shot 2019-08-01 at 11 19 56 PM

Screen Shot 2019-08-01 at 11 22 42 PM

- use https://github.com/TeamWertarbyte/mdi-material-ui icon, like the Tag table does - but use MUI styling, rather than Styled-Components (which is what the Tag table does)

Breaking changes

No

Testing

  1. http://localhost:3000/operator/products - desktop
  2. http://localhost:3000/operator/products - mobile

@machikoyasuda machikoyasuda force-pushed the feat-5392-status-icon branch from 3b7eddd to 47e3a72 Compare August 2, 2019 06:24
Signed-off-by: Machiko Yasuda <[email protected]>
Copy link
Contributor

@cshepherd cshepherd left a comment

Choose a reason for hiding this comment

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

Reviewed code and behavior, all looks great to me!

isVisible: {
color: theme.palette.colors.forestGreen300,
fontSize: theme.typography.fontSize * 1.25,
marginRight: theme.spacing(1)
Copy link
Member

Choose a reason for hiding this comment

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

This is fine, but if the unit is 1 it's not required.

We should probably make a decision on whether to always use a number, even 1, or leave it blank if it's 1.

Doesn't matter to me either way, and will be a quick find / replace whichever way we decide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good to know. Add it to your future Spacing docs :)

@machikoyasuda machikoyasuda merged commit 6ad5f4d into develop Aug 2, 2019
@machikoyasuda machikoyasuda deleted the feat-5392-status-icon branch August 2, 2019 20:52
@kieckhafer kieckhafer mentioned this pull request Aug 28, 2019
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