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

Feature 2795/toggle pulse report #2796

Merged
merged 8 commits into from
Dec 23, 2024
Merged

Conversation

ocielliottc
Copy link
Collaborator

No description provided.

data={pieChartData}
dataKey="value"
nameKey="name"
fill={ociLightBlue}
Copy link
Member

Choose a reason for hiding this comment

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

@ocielliottc Could you color these the same as they are on the bar chart (color from dataInfo)?

See: line 43-45 on https://recharts.org/en-US/examples/PieChartWithCustomizedLabel

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mkimberlin I can. Should we use shades of the blue or orange or the light blue?

Copy link
Member

Choose a reason for hiding this comment

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

Why don't we do two charts? Blue for at work, orange for outside work. Show both if "Both" is selected in the toggle. Then it would feel like it worked similarly to the bar cart.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That works.

{key: "externalDissatisfied", stackId: "external", color: pSBC(-.6, ociOrange), },
{key: "externalNeutral", stackId: "external", color: pSBC(-.4, ociOrange), },
{key: "externalSatisfied", stackId: "external", color: pSBC(-.2, ociOrange), },
{key: "externalVerySatisfied", stackId: "external", color: ociOrange, },
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about going OCI color to Black?

    {key: "internalVeryDissatisfied", stackId: "internal", color: pSBC(-1, ociDarkBlue), },
    {key: "internalDissatisfied", stackId: "internal", color: pSBC(-.75, ociDarkBlue), },
    {key: "internalNeutral", stackId: "internal", color: pSBC(-.5, ociDarkBlue), },
    {key: "internalSatisfied", stackId: "internal", color: pSBC(-.25, ociDarkBlue), },
    {key: "internalVerySatisfied", stackId: "internal", color: ociDarkBlue, },
    {key: "externalVeryDissatisfied", stackId: "external", color: pSBC(-1, ociOrange), },
    {key: "externalDissatisfied", stackId: "external", color: pSBC(-.75, ociOrange), },
    {key: "externalNeutral", stackId: "external", color: pSBC(-.5, ociOrange), },
    {key: "externalSatisfied", stackId: "external", color: pSBC(-.25, ociOrange), },
    {key: "externalVerySatisfied", stackId: "external", color: ociOrange, },

Copy link
Member

Choose a reason for hiding this comment

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

or maybe close to black...like -.9?

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 to -.9

Screenshot 2024-12-23 at 9 01 55 AM

Copy link
Member

@mkimberlin mkimberlin Dec 23, 2024

Choose a reason for hiding this comment

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

Dark mode...
Screenshot 2024-12-23 at 9 04 24 AM

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 guess it catches your eye. The black pie slice looks out of place, to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. .9 is better.

Copy link
Member

Choose a reason for hiding this comment

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

I had a hard time with the original because the gradient went in diff directions meaning the OCI color meant different things in each case. In one case it was bad, the other good. This breaks my brain less. Not sure it's more pleasing, but it's easier to understand, I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, that makes sense! That definitely crossed my mind.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mkimberlin Are you making that change or do you want me to?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, got distracted. I am about to run out the door to pick up my kid, but I can push my changes and resolve the conflict after I drop her and get into the office.

@mkimberlin mkimberlin merged commit 98b2cb7 into develop Dec 23, 2024
5 checks passed
@mkimberlin mkimberlin deleted the feature-2795/toggle-pulse-report branch December 23, 2024 19:28
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.

Need a way to toggle between internal/external/combined in Pulse Reports.
2 participants