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

Add scale to popover. #4628

Closed
Elijbet opened this issue May 26, 2022 · 10 comments
Closed

Add scale to popover. #4628

Elijbet opened this issue May 26, 2022 · 10 comments
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug.

Comments

@Elijbet
Copy link
Contributor

Elijbet commented May 26, 2022

Actual Behavior

Follow up on PR #4627.

Add scale on popover so it's consistent with all other visual components that have a scale and to coordinate with the nested dismissible x icon scale (to ensure visual height consistency for when the icon is collapsed).

Expected Behavior

popover should have a scale prop.

Reproduction Sample

https://developers.arcgis.com/calcite-design-system/components/popover/

Reproduction Steps

Refer to docs to see that popover does not provide a scale prop like other components do.

Reproduction Version

1.0.0-next.478

Relevant Info

No response

Regression?

No response

@Elijbet Elijbet added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels May 26, 2022
@github-actions
Copy link
Contributor

More information is required to proceed with this issue:

This issue will be automatically closed in five days if the information is not provided. Thanks for your understanding.

@github-actions github-actions bot added incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. and removed incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. labels May 26, 2022
@driskull
Copy link
Member

Currently, only scale relationships have existed for components that have a child component. For example, components like dropdown & dropdown-item.

If we want to start having scale relationships with components that are not related in a parent/child sense, then we should probably move scale to a CSS custom property. I don't think we should have unrelated components rely on each other for scale.

I think this might direct us back to this issue: #2868

@Elijbet Elijbet self-assigned this May 31, 2022
@Elijbet
Copy link
Contributor Author

Elijbet commented May 31, 2022

Currently, only scale relationships have existed for components that have a child component. For example, components like dropdown & dropdown-item.

If we want to start having scale relationships with components that are not related in a parent/child sense, then we should probably move scale to a CSS custom property. I don't think we should have unrelated components rely on each other for scale.

I think this might direct us back to this issue: #2868

Thanks for pointing to the cascading scale discussion, that's very helpful.

I edited the issue to leave out the part where I suggest that adding scale would help "accommodate nested items that also have scale, such as an action slot with an icon" as it's a separate issue, even though related.

I was thinking that if all visual components have a scale (this one is missing it by omission rather than by design), whatever is nested within would take the default or user-defined scale of the parent. I’ll read up more on the existing trail of related issues to catch up on the background.

@Elijbet Elijbet assigned jcfranco and unassigned jcfranco May 31, 2022
@jcfranco
Copy link
Member

@Elijbet please correct me if I'm wrong, but this issue stemmed from the popover not having a scale prop to adjust its dismissible button icon accordingly (like we do for other icon-owning components).

I think this might direct us back to this issue: #2868

Agreed. We'll pick up that discussion soon.

@jcfranco
Copy link
Member

I think this might direct us back to this issue: #2868

Agreed. We'll pick up that discussion soon.

We've decided to not support inheriting scale, but we still have the question whether popover needs a scale prop to enable users to set the scale of the dismissible button icon as stated above. Adding @macandcheese @ashetland @SkyeSeitz for their UXpertise™.

@macandcheese
Copy link
Contributor

I'd expect the heading string to also be affected by a scale change, in addition to the close button.

@driskull
Copy link
Member

Agreed ^

@ashetland
Copy link

I'd expect the heading string to also be affected by a scale change, in addition to the close button.

Yep, also agree

@Elijbet Elijbet added 2 - in development Issues that are actively being worked on. and removed 0 - new New issues that need assignment. labels Oct 19, 2022
@Elijbet Elijbet added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 2 - in development Issues that are actively being worked on. labels Oct 31, 2022
@github-actions github-actions bot assigned benelan and geospatialem and unassigned Elijbet Oct 31, 2022
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@jcfranco jcfranco removed the needs triage Planning workflow - pending design/dev review. label Nov 8, 2022
@geospatialem geospatialem added 4 - verified Issues that have been released and confirmed resolved. and removed 3 - installed Issues that have been merged to master branch and are ready for final confirmation. labels Nov 16, 2022
@geospatialem
Copy link
Member

Verified scale in the component's heading and close button.

scale='s':
image

scale='m':
image

scale='l':
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 - verified Issues that have been released and confirmed resolved. bug Bug reports for broken functionality. Issues should include a reproduction of the bug.
Projects
None yet
Development

No branches or pull requests

7 participants