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

Bug/51376 - Support Primer::OpenProject::InputGroup caption #71

Conversation

akabiru
Copy link
Member

@akabiru akabiru commented Jan 29, 2024

What are you trying to accomplish?

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

Support first party caption on the Primer::OpenProject::InputGroup component- providing the caption via the text_input breaks the traling_action layout, hence the system argument is also denied.

Should resolve FIXME: https://github.com/opf/openproject/blob/dev/modules/storages/app/components/storages/admin/forms/redirect_uri_form_component.html.erb#L29-L34

Screenshots

before

Screenshot of Primer Input Group used with flex layout highlighted
Screenshot of broken layout where the trailing action is disjointed from the text input when a caption is provided as a system argument

After
Screenshot of preview with large view port
Screenshot of preview with small view port

Integration

Yes

List the issues that this change affects.

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

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.
  • Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
  • High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

Split out the caption into it's own flex container so it's isolated from the text input + trailing input action

Copy link

changeset-bot bot commented Jan 29, 2024

🦋 Changeset detected

Latest commit: 566408a

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

@akabiru akabiru self-assigned this Jan 29, 2024
@akabiru
Copy link
Member Author

akabiru commented May 24, 2024

Stale- will re-open when working on it

@akabiru akabiru closed this May 24, 2024
@akabiru akabiru deleted the bug/51376-primeropenprojectinputgroup-component-text-input-breaks-with-captions branch May 24, 2024 11:40
@akabiru akabiru restored the bug/51376-primeropenprojectinputgroup-component-text-input-breaks-with-captions branch July 8, 2024 10:49
@akabiru akabiru reopened this Jul 8, 2024
@akabiru akabiru force-pushed the bug/51376-primeropenprojectinputgroup-component-text-input-breaks-with-captions branch from d812d83 to 7b3561f Compare July 8, 2024 10:49
Copy link

github-actions bot commented Jul 8, 2024

Uh oh! @akabiru, the image you shared is missing helpful alt text. Check your pull request body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

@opf opf deleted a comment from github-actions bot Jul 8, 2024
@opf opf deleted a comment from github-actions bot Jul 8, 2024
@akabiru akabiru marked this pull request as ready for review July 8, 2024 14:05
@akabiru akabiru requested a review from HDinger July 8, 2024 14:05
@akabiru akabiru changed the title Bug/51376 - Add Primer::OpenProject::InputGroup captions Bug/51376 - Support Primer::OpenProject::InputGroup captions Jul 8, 2024
@akabiru akabiru changed the title Bug/51376 - Support Primer::OpenProject::InputGroup captions Bug/51376 - Support Primer::OpenProject::InputGroup caption Jul 8, 2024
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.

Works very well! Thanks @akabiru 🥳 I have one minor remark. Further, could you please add the caption to the "playground" preview as well?

app/components/primer/open_project/input_group.html.erb Outdated Show resolved Hide resolved
akabiru and others added 3 commits July 9, 2024 10:49
`Primer::Beta::Text` is _a wrapper component that will apply typography styles to the text inside._

Hence semantically it does not fit the use case, and we'd rather call up the chain.

Co-authored-by: Henriette Darge <[email protected]>
@akabiru akabiru removed their assignment Jul 9, 2024
@akabiru akabiru requested a review from HDinger July 9, 2024 08:41
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.

Awesome, thanks @akabiru 🙇 👍

@HDinger HDinger merged commit b68b3f7 into main Jul 9, 2024
36 checks passed
@HDinger HDinger deleted the bug/51376-primeropenprojectinputgroup-component-text-input-breaks-with-captions branch July 9, 2024 09:23
@openprojectci openprojectci mentioned this pull request Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants