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

OP Heading component in Primer #17879

Merged
merged 4 commits into from
Feb 11, 2025

Conversation

psatyal
Copy link
Contributor

@psatyal psatyal commented Feb 10, 2025

Ticket

https://community.openproject.org/projects/openproject/work_packages/61378/activity

What are you trying to accomplish?

The default Primer Heading component is not sufficiently flexible for use in OpenProject. This implementation aims to create an OP version of it that better integrates with our designs.

Merge checklist

  • Implement in Primer
  • Add code examples to documentation
  • Review code
  • Review LookBook docs
  • Merge

@HDinger HDinger force-pushed the document-primer-op-heading-component branch from 481235a to aa1f02c Compare February 11, 2025 10:49
@HDinger HDinger changed the base branch from dev to release/15.3 February 11, 2025 10:49
Copy link

github-actions bot commented Feb 11, 2025

Caution

The provided work package version does not match the core version

Details:

Please make sure that:

  • The work package version OR your pull request target branch is correct

@HDinger HDinger force-pushed the document-primer-op-heading-component branch from aa1f02c to d23f10c Compare February 11, 2025 10:51
@HDinger HDinger force-pushed the document-primer-op-heading-component branch from d23f10c to 45ed778 Compare February 11, 2025 10:55
Copy link
Contributor Author

@psatyal psatyal left a comment

Choose a reason for hiding this comment

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

Hi @HDinger. The suggested changes look good to me (I can't explicitly approve this because I'm the PR author, but I do in spirit approve it!). I added a little note about this not replacing Primer Heading.

I note that the current implementation does not include the optional divider at the bottom. This request came from @marcalcobe for use in the Account settings page, so I'll mention him here so he's aware of this.

Thanks for the quick work on this!

Cheers
Parimal

@HDinger
Copy link
Contributor

HDinger commented Feb 11, 2025

I note that the current implementation does not include the optional divider at the bottom. This request came from @marcalcobe for use in the Account settings page, so I'll mention him here so he's aware of this.

Just to be clear: This does not mean that it is not possible to add this line, but currently it is simply not implemented yet. With that custom component being available now, we can easily add it later.

@HDinger
Copy link
Contributor

HDinger commented Feb 11, 2025

The failing tests are unrelated.

@HDinger HDinger merged commit 19efaf9 into release/15.3 Feb 11, 2025
11 of 12 checks passed
@HDinger HDinger deleted the document-primer-op-heading-component branch February 11, 2025 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants