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

Support overriding the line height in calcite-block with a css variable #6443

Closed
ethanbdev opened this issue Feb 8, 2023 · 12 comments
Closed
Assignees
Labels
4 - verified Issues that have been released and confirmed resolved. ArcGIS Web Analysis Issues logged by ArcGIS Web Analysis team members bug Bug reports for broken functionality. Issues should include a reproduction of the bug. enhancement Issues tied to a new feature or request. estimate - 2 Small fix or update, may require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library p2 - want for current milestone User set priority status of p2 - want for current milestone

Comments

@ethanbdev
Copy link
Contributor

ethanbdev commented Feb 8, 2023

Description

For some use cases, have received feedback that the line height in the calcite block description text is a bit much:
image
vs
image

It would be nice to customize this via a css variable to give some extra flexibility to fit different amounts of text.

Alternatively, the prop description could accept a <div> or <p> tag

It seems like there might also be some interference with the collabsible property

Acceptance Criteria

Provide a way to override the default line height for the calcite block description text

Relevant Info

No response

Which Component

calcite-block

Example Use Case

No response

Esri team

ArcGIS Web Analysis

@ethanbdev ethanbdev added enhancement Issues tied to a new feature or request. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Feb 8, 2023
@github-actions github-actions bot added the ArcGIS Web Analysis Issues logged by ArcGIS Web Analysis team members label Feb 8, 2023
@alisonailea alisonailea changed the title Support overriding the line height in calcite-block with a css variable Support overriding the line height in calcite-block with a css variable Feb 8, 2023
@macandcheese
Copy link
Contributor

Hi @ethanbdev - is it possible you have conflicting styles or another version CC in play? This is what I'm seeing with various combinations of collapsible and slotted actions in 1.0.3 : https://codepen.io/mac_and_cheese/pen/NWBVmRw?editors=100

Screen Shot 2023-02-08 at 3 45 58 PM

This seems like a place we'd like to standardize on an appropriate line height. cc @ashetland / @SkyeSeitz for design input.

@SkyeSeitz
Copy link

Second that from the Figma side as well. We are already using a 16px line height defined in var(--calcite-font-size--2)

image

So hopefully that is good news, @ethanbdev :)

@ethanbdev
Copy link
Contributor Author

Okay, thanks for the input! Yeah I am seeing some inconsistency trying to apply the line height in different deployments of the components we are building.

Does it seem like then that the second screen shot has too much line height and I need to investigate something?

@macandcheese
Copy link
Contributor

macandcheese commented Feb 9, 2023

It seems like when collapsible is not present - the component becomes susceptible to line-height declarations on parent components:
Screen Shot 2023-02-08 at 3 58 56 PM

We should make sure the component behaves the same in both combinations - I'd prefer to not allow parent line height to affect the style like above.

@macandcheese
Copy link
Contributor

@geospatialem - for triage - issue could be scoped to fixing above. @ethanbdev - is that an ok resolution? I know you mentioned making the description prop a slot for rich content as well.

@ethanbdev
Copy link
Contributor Author

I think if it looks like the screenshot from @SkyeSeitz thats perfect, and I think I was just confused about the collabsible / non-collabsible difference.

As an aside, this looks like NOT the default right? This is what I am trying to fix:
image

@ashetland
Copy link

Correct, the line height in that screenshot doesn't match the default for the component.

@ethanbdev ethanbdev added the p2 - want for current milestone User set priority status of p2 - want for current milestone label Apr 3, 2023
@ethanbdev
Copy link
Contributor Author

Add a p- tag to this.. would be great to standardize the line height getting applied inside the block

@macandcheese macandcheese added the bug Bug reports for broken functionality. Issues should include a reproduction of the bug. label Apr 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023

It looks like this issue is missing some information:

  • A codepen, codesandbox, or jsbin that reproduces the issue. Alternatively, a documentation sample can be used if the issue is reproducible. If you are experiencing build or Node related errors, please provide a GitHub repo for the sample.

Issues without reproduction samples may not be prioritized. If you were unable to create a sample, please try to answer any questions that arise once development begins. Thanks for your understanding.

@macandcheese
Copy link
Contributor

Thanks @ethanbdev - will get this prioritized.

@github-actions github-actions bot added the incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. label Apr 3, 2023
@geospatialem geospatialem added estimate - 2 Small fix or update, may require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Apr 3, 2023
@geospatialem geospatialem removed the incomplete issue report New issues missing important information, and unless provided, they will be closed after 5 days. label Apr 3, 2023
@brittneytewks brittneytewks removed the needs triage Planning workflow - pending design/dev review. label Apr 4, 2023
macandcheese added a commit that referenced this issue Apr 6, 2023
**Related Issue:** #6443 

## Summary
The previous Tailwind text declaration wasn't setting a line-height,
meaning the description could unintentionally inherit line-height from a
parent container. This wasn't affecting the `collapsible` block as the
rendered button has additional style set that prevents the issue.

| Before      | After |
| ----------- | ----------- |
| <img width="460" alt="Screen Shot 2023-04-03 at 5 53 38 PM"
src="https://user-images.githubusercontent.com/4733155/229659153-e9cfb318-1bdc-4778-8545-12ff70d9c110.png">
| <img width="460" alt="Screen Shot 2023-04-03 at 5 53 47 PM"
src="https://user-images.githubusercontent.com/4733155/229659124-a4b4f3a6-b3c6-44d5-a89a-82032cf3d2bc.png">
|
@geospatialem geospatialem added 3 - installed Issues that have been merged to master branch and are ready for final confirmation. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Apr 20, 2023
@github-actions
Copy link
Contributor

Installed and assigned for verification.

@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 Apr 20, 2023
@geospatialem
Copy link
Member

Verified for non-collapsable blocks where the parent has a defined line-height with 1.3.0-next.9 via https://codepen.io/geospatialem/pen/BaqQVpM

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. ArcGIS Web Analysis Issues logged by ArcGIS Web Analysis team members bug Bug reports for broken functionality. Issues should include a reproduction of the bug. enhancement Issues tied to a new feature or request. estimate - 2 Small fix or update, may require updates to tests. p - low Issue is non core or affecting less that 10% of people using the library p2 - want for current milestone User set priority status of p2 - want for current milestone
Projects
None yet
Development

No branches or pull requests

7 participants