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.

Sign up for GitHub

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

Jump to bottom

calcite-accordion-item content padding #4012

Closed
2 tasks
Tracked by #7180
COV-GIS opened this issue Jan 26, 2022 · 9 comments
Closed
2 tasks
Tracked by #7180

calcite-accordion-item content padding #4012

COV-GIS opened this issue Jan 26, 2022 · 9 comments
Assignees
@geospatialem @DitwanP
Labels
4 - verified Issues that have been released and confirmed resolved. design Issues that need design consultation prior to development. design-tokens Issues requiring design tokens. enhancement Issues tied to a new feature or request. estimate - 3 A day or two of work, likely requires updates to tests. p - low Issue is non core or affecting less that 10% of people using the library ready for dev Issues ready for development implementation. visual changes Issues with visual changes that are added for consistency, but are not backwards compatible
Milestone
2024-11-26 - 3.0....

Comments

@COV-GIS
Copy link

COV-GIS commented Jan 26, 2022 •
edited by ashetland
Loading

Description

calcite-accordion-item's shadow dom div[class="accordion-item-content"] has a rendered padding of 0 0.75rem 0.5rem 0.75rem. The top padding of 0 puts any content right up against the bottom of the accordion header and the header focus can overlap the content by a pixel or so.

Acceptance Criteria

Put some space between calcite-accordion-item header and content.
See Calcite Design Solution below

Relevant Info

No response

Which Component

calcite-accordion-item

Example Use Case

No response

Calcite Design Solution

  • Apply default padding to Accordion Item's content slot to match the header padding at each scale.
    Add separate CSS vars for the header area and the content area, per comment
  • Add CSS var to adjust the space around the content slot

Medium scale example:
CleanShot 2024-09-19 at 16 39 19@2x
@COV-GIS COV-GIS added 0 - new New issues that need assignment. enhancement Issues tied to a new feature or request. needs triage Planning workflow - pending design/dev review. labels Jan 26, 2022
@benelan benelan added the design Issues that need design consultation prior to development. label Jun 29, 2022
@paulcpederson
Copy link
Member

I also ran into this. I tried to use the --calcite-accordion-item-padding CSS var but it still doesn't allow adding padding to the top of the content. Further, it also applies the padding to the item's title/header area. Would be nice to make this padding configurable and also allow setting just the content padding.

@geospatialem geospatialem added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Apr 17, 2023
@brittneytewks brittneytewks added the figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists label Sep 12, 2023
@macandcheese macandcheese added the design-tokens Issues requiring design tokens. label Oct 19, 2023
@brittneytewks brittneytewks removed the figma changes Issues that require additions or updates to the Figma UI Kit where no `design` label exists label Dec 11, 2023
@geospatialem geospatialem mentioned this issue Dec 21, 2023
Open
@alisonailea alisonailea mentioned this issue Jan 5, 2024
Closed
@alisonailea alisonailea mentioned this issue Jan 24, 2024
Closed
@ashetland ashetland added the p - low Issue is non core or affecting less that 10% of people using the library label Jul 11, 2024
@ashetland ashetland removed their assignment Jul 11, 2024
@ashetland ashetland added 0 - new New issues that need assignment. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Jul 11, 2024
@ashetland
Copy link

Design recommendations add to description above.

@ashetland ashetland added the ready for dev Issues ready for development implementation. label Jul 26, 2024
@ashetland ashetland removed this from the Design Backlog milestone Jul 26, 2024
@github-actions github-actions bot added the needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. label Jul 26, 2024
@geospatialem
Copy link
Member

This was not addressed in #9861, will need to be a follow-up consideration for design tokens.

@alisonailea alisonailea added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Sep 19, 2024
@alisonailea
Copy link
Contributor

Ah! Got it. Will address ASAP

@geospatialem geospatialem added visual changes Issues with visual changes that are added for consistency, but are not backwards compatible and removed needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. labels Sep 19, 2024
@geospatialem
Copy link
Member

The above will have two different solutions:

  1. Phase I: Component tokens to support user-defined spacing via CSS variables (targeted in September 2024)
  2. Phase II: Default spacing which will alter the component's default spacing. Added the visual changes label as this will have visual changes to the component (targeted in November 2024)

@geospatialem geospatialem added the estimate - 3 A day or two of work, likely requires updates to tests. label Sep 19, 2024
@alisonailea alisonailea added 2 - in development Issues that are actively being worked on. and removed 1 - assigned Issues that are assigned to a sprint and a team member. labels Sep 20, 2024
@alisonailea alisonailea mentioned this issue Nov 13, 2024
Merged
@ashetland
Copy link

Updated the Calcite Design Solution section above. We only need to provide a variable to control the spacing around the content slot, per the original request. This would match our pattern across the system. This comment was misread. If control over the header spacing is also required, a new issue should be filed.

alisonailea added a commit that referenced this issue Nov 18, 2024
@alisonailea
feat(accordion-item): custom header and content spacing tokens (#10721)
b0234f9 
**Related Issue:** #4012 

## Summary

### Add spacing

Removes pt-0 from .content to allow spacing between the header-content
and content.

#### Before

<img width="268" alt="Screenshot 2024-11-13 at 11 30 49 AM"
src="https://github.com/user-attachments/assets/aef62811-5a09-43e1-8328-330cb2c46bc2">

#### After

<img width="267" alt="Screenshot 2024-11-13 at 11 30 59 AM"
src="https://github.com/user-attachments/assets/dd92b276-259e-400b-9fcb-f048b5858426">
alisonailea added a commit that referenced this issue Nov 19, 2024
@alisonailea
feat(accordion-item): custom header and content spacing tokens (#10721)
698072c 
**Related Issue:** #4012 

## Summary

### Add spacing

Removes pt-0 from .content to allow spacing between the header-content
and content.

#### Before

<img width="268" alt="Screenshot 2024-11-13 at 11 30 49 AM"
src="https://github.com/user-attachments/assets/aef62811-5a09-43e1-8328-330cb2c46bc2">

#### After

<img width="267" alt="Screenshot 2024-11-13 at 11 30 59 AM"
src="https://github.com/user-attachments/assets/dd92b276-259e-400b-9fcb-f048b5858426">
@alisonailea alisonailea 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 Nov 21, 2024
@github-actions GitHub Actions
Copy link
Contributor

Installed and assigned for verification.

@github-actions github-actions bot assigned DitwanP and unassigned alisonailea Nov 21, 2024
@DitwanP
Copy link
Contributor

DitwanP commented Nov 21, 2024

🍡 Verified on 3.0.0-next.20

@DitwanP DitwanP closed this as completed Nov 21, 2024
@DitwanP DitwanP 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 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@geospatialem geospatialem

@DitwanP DitwanP

Labels
4 - verified Issues that have been released and confirmed resolved. design Issues that need design consultation prior to development. design-tokens Issues requiring design tokens. enhancement Issues tied to a new feature or request. estimate - 3 A day or two of work, likely requires updates to tests. p - low Issue is non core or affecting less that 10% of people using the library ready for dev Issues ready for development implementation. visual changes Issues with visual changes that are added for consistency, but are not backwards compatible
Projects
None yet
Milestone
2024-11-26 - 3.0.0 Nov Breaking Chang...
Development

No branches or pull requests
9 participants
@paulcpederson @alisonailea @macandcheese @geospatialem @benelan @COV-GIS @DitwanP @ashetland @brittneytewks