feat(RadialProgress): updated with option to render outline around progress indicator #2761

vinnie-olsen · 2023-07-28T15:36:31Z

Overview

Updated RadialProgress component with prop that adds an outline to the progress indicator and prop that adjusts SVG viewbox based on provided stroke-width

PR Checklist

Related to designs: https://www.figma.com/file/Y2xXPfZRbJco3QW0dqsFLx/Module-(Course)-Syllabus-Page?type=design&node-id=196-7717&mode=design&t=8FYMqnPlLeOVWkuZ-0
Related to JIRA ticket: EN-2344
I have run this code to verify it works
This PR includes unit tests for the code change
This PR includes testing instructions tests for the code change
The alpha package of this PR is passing end-to-end tests in all relevant Codecademy repositories

Testing Instructions

Monorepo: See testing instructions on https://github.com/codecademy-engineering/mono/pull/2885

Storybook:

Go to the RadialProgress component and see that there are new progressOutline and adjustForStrokeWidth props available to configure.
Check that the component renders with or without these props and doesn't produce any errors in the console.
Test with a progressOutline, set the prop to an object of the shape {"size": 8, "color": "red"} and see that it renders the outline around the "progress" portion of the component.
Test toggling the adjustForStrokeWidth prop on and off. While it's off (false), any stroke-width above 10 will be cut off by the viewport of the SVG. This is the existing behavior (prior to these changes) and will also happen if the strokeWidth + progressOutline.size is greater than 10 when added together. However when the adjustForStrokeWidth prop is toggled on (true), it will adjust for stroke-widths greater than 10 and will ensure the entire progress circle fits within the SVG viewport without being cut off.
Test adding a baseColor prop with a custom color. When this is added, it should render the "non-progress" part of the circle in the provided custom color (in a solid/"1" opacity). When this prop is not present, it will work as it did prior to these changes and will render the "non-progress" part of the circle in the provided color prop value but with "0.2" opacity.

PR Links and Envs

Repository	PR Link	PR Env
Monolith	Monolith PR
Portal	Portal Link	Portal Env

…indicator

jakemhiller · 2023-08-04T17:36:10Z

packages/gamut/src/RadialProgress/index.tsx

+    size: number;
+    color: string;
+  };
+  adjustForStrokeWidth?: boolean;


i'm probably missing something, but i'm wondering why we'd want this to be false?

I kind of went back and forth on that actually. I mainly ended up having it as a prop to help reduce the chance of regressions with any other implementations of this component and also to be able to easily toggle it off if it causes any issues. I did check the prod storybook though and this issue of the circle being cut off by the SVG viewport when the stroke-width is larger than 10 was present prior to any of the changes I made on this branch.

I'd be open to removing the prop and just having that logic always apply if you think that would be preferable though. What are your thoughts?

yeah i think that logic can apply to all RadialProgress components, it shouldn't really be possible to go outside of those bounds in the first place

Gotcha yep I can make that change 👍

@jakemhiller I made that change

jakemhiller · 2023-08-04T18:18:39Z

packages/gamut/src/RadialProgress/index.tsx

+  progressOutline?: {
+    size: number;
+    color: string;
+  };


how do you feel about this being two separate properties, to match the other width/color properties?

So like separate progressOutlineSize and progressOutlineColor props instead of the progressOutline object with size and color properties?

i wonder if you could also set a reasonable default for the color? it seems like it would have to be changed in a lot of situations, so maybe not, but it seems odd if you can set the size and not see the outline

Yeah agreed and that was the main reason that I set the progressOutline to be an object with both the size and color properties required. So basically if you use a progressOutline prop at all, those properties both need to be present. Alternatively, I suppose I could set the progressOutlineColor to be black by default. I think black is the default color for the "progress" part of the circle if you don't use a color prop on the RadialProgress component.

@jakemhiller I made these changes. Lmk if that looks like what you had in mind. I also set the default progressOutlineColor to be black.

I haven't messed with the ColorMode stuff in gamut for a while, but after looking at it a bit i think what you want is theme.colors['background-current']. that should mean most folks won't need to change that prop, it will match whatever background as long as the background was set using gamut.

…dial-progress-outline

jakemhiller · 2023-08-07T18:25:51Z

packages/gamut/src/RadialProgress/index.tsx

@@ -34,10 +38,45 @@ export const RadialProgress: React.FC<RadialProgressProps> = ({
  value,
  strokeLinecap = 'round',
  strokeWidth = 10,
+  progressOutlineSize,
+  progressOutlineColor = theme.colors.black,


just b/c our previous conversation was resolved. i think this could be theme.colors['background-current'] so that it automatically matches the background 👍

Gotcha I'll change it over to theme.colors['background-current'] then - thanks!

jakemhiller

after updating the outline color again I think you'll be good to go.

I was wondering whether progressOutlineSize could actually be a % based on the size of the progress circle, since i think design probably has a set size in mind, but i don't think that's a blocker

vinnie-olsen · 2023-08-07T19:05:03Z

@jakemhiller awesome thanks!

Regarding the progressOutlineSize being a percentage, I'm not sure if it would work with the way this solution is currently setup since the stroke width of the "outline" circle is being computed using the strokeWidth prop together with the progressOutlineSize prop. For that reason, I think they would need to be the same relative units. It's possible that I'm misunderstanding though. Unless you have a strong opinion on making that change, I might just keep it as-is.

jakemhiller · 2023-08-07T19:09:18Z

@vinnie-olsen no, that makes sense, lets go with what you have 👍

… RadialProgress component

codecademydev · 2023-08-07T21:41:45Z

📬Published Alpha Packages:

@codecademy/[email protected]
@codecademy/[email protected]
@codecademy/[email protected]
@codecademy/[email protected]

codecademydev · 2023-08-07T21:42:57Z

🚀 Styleguide deploy preview ready!

https://64d1655bf945ab3eeae6f78f--gamut-preview.netlify.app

Deploy Logs

vinnie-olsen and others added 6 commits July 28, 2023 09:40

updated RadialProgress with option to render outline around progress …

2a00831

…indicator

Merge branch 'main' into vo_en-2344_radial-progress-outline

3a7d4d0

Merge branch 'main' into vo_en-2344_radial-progress-outline

2b500d3

Modified RadialProgress to optionally adjust for excess stroke width

0a5a81f

Merge branch 'main' into vo_en-2344_radial-progress-outline

e1a6ec2

Merge branch 'main' into vo_en-2344_radial-progress-outline

156a26d

vinnie-olsen marked this pull request as ready for review August 3, 2023 16:04

vinnie-olsen requested review from a team, jakemhiller, dreamwasp, antoniablair, CharlieBliss and vonbarown and removed request for a team August 3, 2023 16:04

Merge branch 'main' into vo_en-2344_radial-progress-outline

d374799

vinnie-olsen removed request for jakemhiller, dreamwasp, antoniablair, CharlieBliss and vonbarown August 3, 2023 20:54

Added 'baseColor' prop/logic to 'RadialProgress' component

a7e2fd9

vinnie-olsen requested review from a team, aresnik11, dreamwasp, antoniablair, CharlieBliss and vonbarown and removed request for a team August 4, 2023 17:29

jakemhiller reviewed Aug 4, 2023

View reviewed changes

vonbarown approved these changes Aug 4, 2023

View reviewed changes

jakemhiller reviewed Aug 4, 2023

View reviewed changes

vinnie-olsen added 2 commits August 7, 2023 10:02

Refactored RadialProgress progressOutline props

65175ce

Merge branch 'main' of github.com:Codecademy/gamut into vo_en-2344_ra…

12e3521

…dial-progress-outline

jakemhiller reviewed Aug 7, 2023

View reviewed changes

jakemhiller approved these changes Aug 7, 2023

View reviewed changes

Changed progressOutlineColor prop default to 'background-current' for…

59a2a78

… RadialProgress component

vinnie-olsen added the Ship It Automerge this PR when possible label Aug 7, 2023

codecademydev merged commit 442a6aa into main Aug 7, 2023
11 of 12 checks passed

codecademydev deleted the vo_en-2344_radial-progress-outline branch August 7, 2023 21:53

codecademydev removed the Ship It Automerge this PR when possible label Aug 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(RadialProgress): updated with option to render outline around progress indicator #2761

feat(RadialProgress): updated with option to render outline around progress indicator #2761

vinnie-olsen commented Jul 28, 2023 •

edited

Loading

jakemhiller Aug 4, 2023

vinnie-olsen Aug 4, 2023

jakemhiller Aug 4, 2023

vinnie-olsen Aug 4, 2023

vinnie-olsen Aug 7, 2023

jakemhiller Aug 4, 2023

vinnie-olsen Aug 4, 2023

jakemhiller Aug 4, 2023

jakemhiller Aug 4, 2023

vinnie-olsen Aug 4, 2023 •

edited

Loading

vinnie-olsen Aug 7, 2023

jakemhiller Aug 7, 2023

jakemhiller Aug 7, 2023

vinnie-olsen Aug 7, 2023

jakemhiller left a comment

vinnie-olsen commented Aug 7, 2023

jakemhiller commented Aug 7, 2023

codecademydev commented Aug 7, 2023

codecademydev commented Aug 7, 2023

feat(RadialProgress): updated with option to render outline around progress indicator #2761

feat(RadialProgress): updated with option to render outline around progress indicator #2761

Conversation

vinnie-olsen commented Jul 28, 2023 • edited Loading

Overview

PR Checklist

Testing Instructions

PR Links and Envs

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinnie-olsen Aug 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakemhiller left a comment

Choose a reason for hiding this comment

vinnie-olsen commented Aug 7, 2023

jakemhiller commented Aug 7, 2023

codecademydev commented Aug 7, 2023

📬Published Alpha Packages:

codecademydev commented Aug 7, 2023

vinnie-olsen commented Jul 28, 2023 •

edited

Loading

vinnie-olsen Aug 4, 2023 •

edited

Loading