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

Allow setting an id attribute on the BorderBox's ul element #57

Merged
merged 3 commits into from
Nov 28, 2023

Conversation

aaron-contreras
Copy link

What are you trying to accomplish?

In order to perform turbo stream append and prepend actions on a BorderBox component, we need a way to specify a target via an id (or some unique CSS selector). We can't guarantee there is a unique selector for the specific BorderBox's ul element we want to append to without an id.

This commit allows providing the id that will be rendered in the BorderBox's ul element via the list_id system argument.

A system argument has been added to the "Playground" preview for the BorderBox.

Screenshots

Before

Before picture

After

After picture

Integration

No

List the issues that this change affects.

https://community.openproject.org/wp/51277

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

In order to perform turbo stream append and prepend actions on a
`BorderBox` component, we need a way to specify a target via an `id`
(or some unique CSS selector). We can't guarantee there is a unique
selector for the specific BorderBox's `ul` element we want to append
to without an id.

This commit allows providing the `id` that will be rendered in the
`BorderBox`'s ul element via the `list_id` system argument.

A system argument has been added to the "Playground" preview for the
`BorderBox`.
Copy link

changeset-bot bot commented Nov 27, 2023

🦋 Changeset detected

Latest commit: 29a18c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@openproject/primer-view-components Patch

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

@aaron-contreras aaron-contreras force-pushed the housekeeping/allow-id-as-list-argument-on-border-boxes branch from e8fef1a to 7e1ca2b Compare November 27, 2023 22:23
Copy link
Collaborator

@HDinger HDinger left a comment

Choose a reason for hiding this comment

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

Lgtm 👍

@HDinger HDinger force-pushed the housekeeping/allow-id-as-list-argument-on-border-boxes branch from 51d251f to 29a18c9 Compare November 28, 2023 08:40
@opf opf deleted a comment from github-actions bot Nov 28, 2023
@HDinger HDinger merged commit 31594bd into main Nov 28, 2023
55 of 56 checks passed
@HDinger HDinger deleted the housekeeping/allow-id-as-list-argument-on-border-boxes branch November 28, 2023 08:58
@openprojectci openprojectci mentioned this pull request Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants