Skip to content
This repository has been archived by the owner on Jun 29, 2023. It is now read-only.

feat(action-bar,action-pad): Add tooltipExpand property #904

Merged
merged 7 commits into from
Apr 2, 2020

Conversation

driskull
Copy link
Member

@driskull driskull commented Apr 1, 2020

Related Issue: None

Summary

  • Add a expandTooltip property to calcite-action-bar and calcite-action-pad to show a HTMLCalciteTooltipElement when the action is collapsed.

@driskull driskull self-assigned this Apr 1, 2020
@driskull driskull added 1 - assigned enhancement New feature request for an existing component labels Apr 1, 2020
@driskull driskull added this to the KOO (King of Ooo) milestone Apr 1, 2020
@driskull driskull marked this pull request as ready for review April 1, 2020 18:49
@driskull driskull requested a review from a team as a code owner April 1, 2020 18:49
@driskull driskull changed the title feat(action-bar,action-pad): Allow access to toggle action feat(action-bar,action-pad): Add expandTooltip property Apr 1, 2020
Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

Code looks good, but I have a few questions:

  1. Does this mean we have to add a similar prop for any component setting the title attr?

  2. If this is a pattern we'll be applying across components, should we consider using tooltip as the prefix? This would be consistent w/ intl props.

  3. Could you also update the component's stories? 📚

src/components/calcite-action-bar/calcite-action-bar.tsx Outdated Show resolved Hide resolved
@driskull
Copy link
Member Author

driskull commented Apr 1, 2020

Does this mean we have to add a similar prop for any component setting the title attr?

@jcfranco I don't think we have to for every component but I do see the map viewer team asking for more options to include tooltips for shadowed elements so we should have a plan for this incase.

If this is a pattern we'll be applying across components, should we consider using tooltip as the prefix? This would be consistent w/ intl props.

Yeah we should consider it. Would tooltipExpand be ok?

@driskull
Copy link
Member Author

driskull commented Apr 1, 2020

I'll work on a couple basic tests and stories.

@driskull
Copy link
Member Author

driskull commented Apr 1, 2020

@jcfranco added usage file but can't figure out how to add story with a prop that accepts a dom node.

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

added usage file but can't figure out how to add story with a prop that accepts a dom node.

Can you create a follow-up issue for this?

@driskull driskull changed the title feat(action-bar,action-pad): Add expandTooltip property feat(action-bar,action-pad): Add tooltipExpand property Apr 2, 2020
@driskull driskull merged commit 94476e9 into master Apr 2, 2020
@driskull driskull deleted the dris0000/action-tooltip branch April 2, 2020 18:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 - assigned enhancement New feature request for an existing component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants