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

Progress Bar Component #99

Merged
merged 17 commits into from
Nov 30, 2023
Merged
50 changes: 50 additions & 0 deletions src/components/presentational/ProgressBar/ProgressBar.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
.progress-bar {
Copy link
Contributor

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

Copy link
Contributor

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)

display:inline-block;
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would remove the margin on this

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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) ;
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

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

i would probably drop the border because it makes it look a bit awkward when stuff overlaps it:
Screenshot 2023-11-30 at 2 04 36 PM

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)
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

@chrisnowak chrisnowak Nov 28, 2023

Choose a reason for hiding this comment

The 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 {
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

  • borderColor?: ColorDefinition
  • backgroundColor: ColorDefinition
  • children (this is where you'd put in the font awesome icon for the star, or the text 75 or whatnot)
  • height (based on this it would auto-calculate the top margin to center itself vertically, e.g. for the large star)

e.g. for the big star at the end your step would be like:

steps: [{
		stepPercent: 100,
		stepIcon: <ProgressBarStep borderColor="yellow" backgroundColor="orange" height="24px">
<FontAwesomeIcon icon={faStar} style={{ color: "yellow" }} />
</ProgressBarStep>
	}],

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;
}
102 changes: 102 additions & 0 deletions src/components/presentational/ProgressBar/ProgressBar.stories.tsx
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" },
Copy link
Contributor

Choose a reason for hiding this comment

The 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>;
Copy link
Contributor

Choose a reason for hiding this comment

The 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



41 changes: 41 additions & 0 deletions src/components/presentational/ProgressBar/ProgressBar.tsx
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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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)

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor

Choose a reason for hiding this comment

The 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;
Copy link
Contributor

Choose a reason for hiding this comment

The 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",
Copy link
Contributor

Choose a reason for hiding this comment

The 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",
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably have the default fill be var(--mdhui-color-primary)

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>
);
}
1 change: 1 addition & 0 deletions src/components/presentational/ProgressBar/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as ProgressBar } from './ProgressBar'
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to also export this from presentational/index.ts