-
Notifications
You must be signed in to change notification settings - Fork 2
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
Progress Bar Component #99
Changes from 7 commits
54694b3
39256e9
701587d
2fd549d
b953e02
7efe1cd
ecd9297
e655b7f
7b82f4d
fe5846a
b285a41
cd672b0
a08c901
e36ea9b
7240ae7
512ff86
dbe3a4f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
.progress-bar { | ||
display:inline-block; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why inline-block? I think we would want this to be block? |
||
margin: 10px 0px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would remove the margin on this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. removed. I realize now that the reason this was there was to compensate for the way storybook is set to hide overflow. |
||
width: 100%; | ||
max-width: 500px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why a max-width? Will look awkward on desktop so I would remove |
||
--icon_size: 24px; | ||
--icon_border_width: 2px; | ||
} | ||
|
||
.progress-bar-fill { | ||
position: absolute; | ||
min-height: 1px; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like some autoformatting is off in this file |
||
height: calc( var(--icon_size) + 4px); | ||
top:0; | ||
border-radius: var(--icon_size); | ||
z-index:1 | ||
} | ||
|
||
.progress-bar .progress-bar-background { | ||
border-radius: 500px; | ||
border-width: var(--icon_border_width) ; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh...did you just add a border to this? I didn't think there was a border before There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the border width is added to the width so it ends up overflowing 100% width There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
border-style: none; | ||
background: #fff; | ||
overflow: hidden; | ||
width: 100%; | ||
position: relative; | ||
height:calc( var(--icon_size) + 4px); | ||
z-index:1; | ||
} | ||
|
||
.progress-bar .progress-steps { | ||
top:0; | ||
margin: 10px 0px; | ||
width: 100%; | ||
max-width: 500px; | ||
position: absolute; | ||
z-index: 2; | ||
display:inline-block; | ||
} | ||
|
||
.progress-bar .step-icon { | ||
position: absolute; | ||
transform: translate(-100%, 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does make things a bit awkward if you have a stop near the beginning of the progress bar - it will get clipped out...there's no way to have a stop at 0 reliably I think (since steps are based on percent). That being said, this seems like the best and most reasonable solution to this and, as demonstrated by the stories, works pretty well. I don't think having a stop right at the beginning makes a ton of sense There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and someone using this could always fix that by having the element at the beginning have a left margin or something |
||
} | ||
|
||
.progress-bar .step-icon span { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't assume a span will be used as the child element... So there is actually some cool stuff you did in the story here for the various little star icons and how they are colored etc. I would not just waste that in a story but would actually invest in a separate component called "ProgressBarStep"...it could be exported under the progress bar...such that we have some out of box progress bar steps that can be placed on this (but someone could also come up with their own to supply to this). and the props could be like:
e.g. for the big star at the end your step would be like:
Basically to have to limit the amount of work someone has to do to get it to look like the storybook, while still allowing some flexibility |
||
display: flex; | ||
align-items: center; | ||
justify-content: center; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,102 @@ | ||
import React from "react"; | ||
import ProgressBar, { ProgressBarProps } from "./ProgressBar"; | ||
import { Meta, StoryFn } from "@storybook/react"; | ||
import Layout from "../Layout"; | ||
import { faStar } from "@fortawesome/free-regular-svg-icons"; | ||
import { faStar as faSolidStar } from "@fortawesome/free-solid-svg-icons"; | ||
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome"; | ||
|
||
export default { | ||
title: "Presentational/ProgressBar", | ||
component: ProgressBar, | ||
parameters: { | ||
layout: 'auto', | ||
}, | ||
argTypes: { | ||
background: { control: 'color', description: "The background color of the progress bar" }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Documentation! Awesome. Need to start doing that... |
||
fill: { control: 'color', description: "The fill color of the progress bar" }, | ||
fillPercent: { control: 'number', description: "The percent of the progress bar that is filled" }, | ||
steps: { control: 'object', description: "An array of steps to display on the progress bar. A step specifies the position on the progress bar as a percent and a React element to display at that position." }, | ||
} | ||
} as Meta<typeof ProgressBar>; | ||
|
||
const Template: StoryFn<typeof ProgressBar> = (args: ProgressBarProps) => <Layout colorScheme="auto"><ProgressBar {...args} /></Layout>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor, maybe put "defaultMargin" on this so it's spaced out from the edges |
||
|
||
export const Default = Template.bind({}); | ||
Default.args = { | ||
fillPercent: 35 | ||
} | ||
|
||
export const GradientFill75TargetStep = Template.bind({}); | ||
GradientFill75TargetStep.args = { | ||
fillPercent: 75, | ||
fill: "linear-gradient(90deg, #6cb144, #adc247)", | ||
steps: [{ | ||
stepPercent: 100, | ||
stepIcon: | ||
<span style={{ borderRadius: "24px", height: "24px", width: "24px", marginTop: "-2px", color: "rgba(148, 148, 148, 1)", backgroundColor: "rgba(148, 148, 148, 1)", padding: "4px 4px" }}> | ||
<FontAwesomeIcon icon={faStar} size={"1x"} style={{ color: "#fcfcfc", marginTop: "-3px" }} /> | ||
</span> | ||
}], | ||
} as ProgressBarProps; | ||
|
||
export const GradientFill100TargetStep = Template.bind({}); | ||
GradientFill100TargetStep.args = { | ||
fillPercent: 100, | ||
fill: "linear-gradient(90deg, #6cb144, #adc247)", | ||
steps: [{ | ||
stepPercent: 100, | ||
stepIcon: | ||
<span style={{ border: "2px solid gold", borderRadius: "24px", height: "24px", width: "24px", marginTop: "-3px", color: "rgba(148, 148, 148, 1)", backgroundColor: "#EA6B54", padding: "4px 4px" }}> | ||
<FontAwesomeIcon icon={faSolidStar} size={"1x"} style={{ color: "gold", marginTop: "-3px" }} /> | ||
</span> | ||
}], | ||
} as ProgressBarProps; | ||
|
||
export const GradientFillIconSteps = Template.bind({}); | ||
GradientFillIconSteps.args = { | ||
fillPercent: 50, | ||
fill: "linear-gradient(270deg, #F0CA68 0%, #E5917F 100%)", | ||
steps: [...Array(4)].map((_e, i) => { | ||
let value = (i + 1) * 25; | ||
let icon = (value <= 50) ? | ||
<span style={{ border: "2px solid gold", borderRadius: "16px", height: "16px", width: "16px", color: "rgba(148, 148, 148, 1)", backgroundColor: "#EA6B54", padding: "4px 4px" }}> | ||
<FontAwesomeIcon icon={faSolidStar} size={"1x"} style={{ color: "white", marginTop: "-3px" }} /> | ||
</span> : | ||
<span style={{ border: "2px solid rgba(148, 148, 148, 1)", borderRadius: "16px", height: "16px", width: "16px", color: "rgba(148, 148, 148, 1)", backgroundColor: "rgba(148, 148, 148, 1)", padding: "4px 4px" }}> | ||
<FontAwesomeIcon icon={faSolidStar} size={"1x"} style={{ color: "white", marginTop: "-3px" }} /> | ||
</span>; | ||
return { | ||
stepPercent: Math.trunc(value), | ||
stepIcon: icon | ||
} | ||
}) | ||
} as ProgressBarProps; | ||
|
||
export const LabelSteps = Template.bind({}); | ||
LabelSteps.args = { | ||
fillPercent: 100 * 75 / 175, | ||
fill: "rgb(34, 115, 209, 0.5)", | ||
steps: [...Array(7)].map((_e, i) => { | ||
let amount = (i + 1) * 25; | ||
let icon = (amount == 75) ? | ||
<span style={{ border: "2px solid white", borderRadius: "14px", height: "14px", width: "14px", color: "white", backgroundColor: "rgb(34, 115, 209)", padding: "4px 4px", fontSize: "0.8em", marginTop: "1px" }}> | ||
75 | ||
</span> : | ||
((amount < 75) ? | ||
<span style={{ border: "2px solid white", borderRadius: "14px", height: "14px", width: "14px", color: "rgba(148, 148, 148, 1)", backgroundColor: "rgb(34, 115, 209)", padding: "4px 4px", marginTop: "1px" }}> | ||
<FontAwesomeIcon icon={faSolidStar} size={"1x"} style={{ color: "white", marginTop: "-3px" }} /> | ||
</span> : | ||
<span style={{ border: "2px solid rgba(148, 148, 148, 1)", borderRadius: "14px", height: "14px", width: "14px", color: "rgba(148, 148, 148, 1)", backgroundColor: "rgba(148, 148, 148, 1)", padding: "4px 4px", marginTop: "1px" }}> | ||
<FontAwesomeIcon icon={faStar} size={"1x"} style={{ color: "rgb(34, 115, 209)", marginTop: "-3px" }} /> | ||
</span> | ||
); | ||
return { | ||
stepPercent: Math.trunc(100 * amount / 175), | ||
stepIcon: icon | ||
} | ||
}) | ||
} as ProgressBarProps | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
import "./ProgressBar.css"; | ||
import React, { ReactElement, useEffect } from "react"; | ||
|
||
export interface ProgressBarProps { | ||
background?: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We use color suffix for color properties, so would use backgroundColor and fillColor. Also would make both background/fill a ColorDefinition and not a string - that was probably introduced after this PR but see the button component for an example (ColorDefinition allows specifying 2 colors, one for light mode and one for dark mode) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh hmm, i'm realizing that fill might actually be a gradient...yet let me think about that for a sec There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nevermind, I think it does't matter too much actually. Should still use ColorDefinition which just ends up being 2 strings so you could specify 2 gradients theoretically. Currently view builder doesn't let you configure a gradient but that's something we could look at adding |
||
fill?: string; | ||
fillPercent: number; | ||
chrisnowak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
steps?: [ | ||
{ | ||
stepPercent: number; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "step" prefix here seems unnecessary since this will be in a "steps" array; so would just go with percent/icon |
||
stepIcon: ReactElement; | ||
} | ||
] | ||
} | ||
|
||
ProgressBar.defaultProps = { | ||
chrisnowak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
background: "#d3d3d3", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would have this be the closest CSS backround color var from the globalCss file |
||
fill: "#00A862", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would probably have the default fill be |
||
fillPercent: 0 | ||
} | ||
|
||
export default function ProgressBar(props: ProgressBarProps) { | ||
return ( | ||
<div className="progress-bar"> | ||
<div className="progress-bar-background" style={{ background: props.background }}> | ||
<div className="progress-bar-fill" style={{ width: props.fillPercent + "%", background: props.fill }} /> | ||
</div> | ||
<div className="progress-steps"> | ||
{props.steps && | ||
<> | ||
{props.steps.map((step, i) => | ||
<div key={`${i}-step`} className="step-icon" style={{ left: step.stepPercent + "%" }}> | ||
{step.stepIcon} | ||
</div> | ||
)} | ||
</> | ||
} | ||
</div> | ||
</div> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default as ProgressBar } from './ProgressBar' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You'll want to also export this from presentational/index.ts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should prefix all CSS classes with .mdhui-, so this should be .mdhui-progress-bar
I would also nest all classes in this file under that - so for the fill it shuold be
.mdhui-progress-bar .mdhui-progress-bar-fill
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(alternative to consider at some point is using CSS modules; but so far we haven't taken that leap and CSS is still being overridden in places)