Skip to content

A few minor component updates.#605

Open
greinard wants to merge 1 commit into
mainfrom
MinorUpdates
Open

A few minor component updates.#605
greinard wants to merge 1 commit into
mainfrom
MinorUpdates

Conversation

@greinard
Copy link
Copy Markdown
Collaborator

Overview

This branch has a few minor updates to existing components.

  • Card now supports having a click handler. This is useful when you want to display a number child elements in a card, but have the entire thing function as a single CTA. When a click handler is provided, the cursor gets set to pointer.
  • Action now inherits the cursor from its parent rather than setting it to default. This allows a clickable card's cursor to pass through to child actions when that child action does not have a click handler of its own. If the action has a click handler of its own, its cursor still gets set to pointer regardless of what the parent cursor is. I also cleaned up and modernized the Action Storybook a bit.
  • Goal was updated such that it also inherits the cursor from its parent. The recharts-wrapper class was setting the cursor to default, but now it inherits the cursor. This ensures the cursor is uniform across the goal rather than swapping back and forth to default when pieces of the chart are hovered.
  • Goal was also updated to prevent the outline from showing up on clicked pie chart segments in the progress ring.

Security

  • I have ensured no secure credentials or sensitive information remain in code, metadata, comments, etc.
    • Please verify that you double checked that .storybook/preview.js does not contain your participant access key details.
    • There are no temporary testing changes committed such as API base URLs, access tokens, print/log statements, etc.
  • These changes do not introduce any security risks, or any such risks have been properly mitigated.

No new security risk. Just minor rendering and event handler updates.

Testing

  • This change can be adequately tested using the MDH Storybook.
  • This change requires additional testing in the MDH iOS/Android/Web apps. (Create a pre-release tag/build and test in a ViewBuilder PR.)

I have updated the Storybook stories to make it fairly simple to verify the above claims.

Documentation

  • I have added relevant Storybook updates to this PR.
  • If this feature requires a developer doc update, I have tagged @CareEvolution/api-docs.
  • This change does not impact documentation or Storybook.

@aws-amplify-us-east-1
Copy link
Copy Markdown

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-605.d1xp2kmk6zrv44.amplifyapp.com

<Goal variant="compact" label="Goal 2" targetValue={5} maxValue={10} valueProvider={{ type: 'static integer', getValue: async () => 3 }} />
</div>
</>,
onClick: () => console.log("Card Clicked!")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"See more details" made me expect to see more details, not a console statement. I think it would be better if the click event actually did something visible (even if it was as simple as changing a color or something) with a prompt to match.


return (
<div style={{...props.style, backgroundColor:backgroundColor}} ref={props.innerRef} className={classes.join(" ")}>
<div style={{ ...props.style, backgroundColor: backgroundColor, ...(props.onClick && { cursor: "pointer" }) }} ref={props.innerRef} className={classes.join(" ")} onClick={props.onClick}>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In Chrome and Safari on Mac, I don't see any visible difference between the pointer on a clickable card vs a non-clickable card.

colorScheme: 'auto',
title: 'Baseline Survey',
subtitle: 'Tap here to start your baseline survey',
withClass: false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe I'm missing something, but the story args here seem a little inconsistent compared to other stories. I'm used to seeing the actual component properties reflected in the story arguments. For example, on BasicPointsForBadges you can control the BasicPointsForBadgesProps like pointsPerBadge and progressBarColor. Here the story args seem to be activating specific scenarios, but I thought that was more normally done with separate stories?

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.

2 participants