-
Notifications
You must be signed in to change notification settings - Fork 1
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
[62577] Create a BorderBox::CollapsibleHeader
component
#260
[62577] Create a BorderBox::CollapsibleHeader
component
#260
Conversation
🦋 Changeset detectedLatest commit: 3a1eb7f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
BorderBox::CollapsibleHeader
component
423341b
to
3d42a7f
Compare
364dec0
to
d278433
Compare
d278433
to
18d5201
Compare
18d5201
to
0111e1d
Compare
edc5e5a
to
c718605
Compare
|
app/components/primer/open_project/border_box/collapsible_header.rb
Outdated
Show resolved
Hide resolved
app/components/primer/open_project/border_box/collapsible_header.rb
Outdated
Show resolved
Hide resolved
app/components/primer/open_project/border_box/collapsible_header.rb
Outdated
Show resolved
Hide resolved
app/components/primer/open_project/border_box/collapsible_header.ts
Outdated
Show resolved
Hide resolved
app/components/primer/open_project/border_box/collapsible_header.ts
Outdated
Show resolved
Hide resolved
previews/primer/open_project/border_box/collapsible_header_preview.rb
Outdated
Show resolved
Hide resolved
previews/primer/open_project/border_box/collapsible_header_preview.rb
Outdated
Show resolved
Hide resolved
test/components/primer/open_project/border_box/collapsible_header_test.rb
Outdated
Show resolved
Hide resolved
test/components/primer/open_project/border_box/collapsible_header_test.rb
Outdated
Show resolved
Hide resolved
# @label Default | ||
# @snapshot | ||
def default | ||
render_with_template | ||
end | ||
|
||
# @label With counter | ||
# @snapshot | ||
def with_count | ||
render_with_template | ||
end | ||
|
||
# @label With description text | ||
# @snapshot | ||
def with_description | ||
render_with_template | ||
end | ||
|
||
# @label Collapsed initially | ||
# @snapshot | ||
def collapsed | ||
render_with_template | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the template files that you added are basically the same. So either you provide different BorderBox examples in each or they should all use the same template with different variables, e.g
# @label Default
# @snapshot
def default
render_with_template(
template: "primer/open_project/border_box/collapsible_header_preview/playground",
locals: {
title: "Default title",
description: nil,
count: nil,
collapsed: false
}
)
end
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I redid them all with just the playground template
1af0473
to
2a79d0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Thanks for all the effort 🙇
Thanks for all the education and help :) |
What are you trying to accomplish?
Screenshots
Integration
List the issues that this change affects.
https://community.openproject.org/projects/design-system/work_packages/62577/activity
Risk Assessment
What approach did you choose and why?
Anything you want to highlight for special attention from reviewers?
Accessibility
Merge checklist