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

[PLAY-1581] Height, minHeight, maxHeight for Card, Flex, FlexItem, & Dialog #3749

Merged
merged 12 commits into from
Oct 22, 2024

Conversation

jasperfurniss
Copy link
Contributor

@jasperfurniss jasperfurniss commented Oct 1, 2024

What does this PR do? A clear and concise description with your runway ticket url.
https://runway.powerhrg.com/backlog_items/PLAY-1581

In order to achieve the height functionality that the story demanded, we introduced a new utility function for handling specific global properties that require dynamic values (any px, %, or vh, etc). This has been on our Todo list for a looooong time, and I'm so happy we were able to knock out the work here in this story!

The work in this PR adds the following props for react: minHeight, maxHeight, height for React
The work in this PR adds the following props for rails: min_height, max_height, height for Rails

For now, these are applied to a few of our utility kits: Card, Flex, FlexItem, Dialog

We have a path forward to apply to all other kits, but for now this work unblocks the Template work for the PB-In-Nitro team, without shipping changes to every kit which would move this to a much higher risk-level.

Screenshots: Screenshots to visualize your addition/change

How to test? Steps to confirm the desired behavior:

  1. Go to dialog, flex, flex_item, or card.
  2. Add a height prop with a value.
  3. See your styles get applied.

Checklist:

  • LABELS Add a label: enhancement, bug, improvement, new kit, deprecated, or breaking. See Changelog & Labels for details.
  • DEPLOY I have added the milano label to show I'm ready for a review.
  • TESTS I have added test coverage to my code.

@jasperfurniss jasperfurniss self-assigned this Oct 2, 2024
@jasperfurniss jasperfurniss marked this pull request as ready for review October 14, 2024 18:39
@jasperfurniss jasperfurniss requested review from a team as code owners October 14, 2024 18:39
@jasperfurniss jasperfurniss changed the title Initial Height Work Height, minHeight, maxHeight for Card, Flex, FlexItem, & Dialog Oct 14, 2024
@jasperfurniss jasperfurniss changed the title Height, minHeight, maxHeight for Card, Flex, FlexItem, & Dialog [PLAY-1581] Height, minHeight, maxHeight for Card, Flex, FlexItem, & Dialog Oct 14, 2024
@jasperfurniss jasperfurniss added the milano 20 MAX - Deploy this PR to a review environment via Milano label Oct 14, 2024
Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.5.0.pre.alpha.PLAY15814040

Your Alpha for NPM is 14.5.0-alpha.PLAY15814040

Copy link
Contributor

@kangaree kangaree left a comment

Choose a reason for hiding this comment

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

@jasperfurniss

Since we're adding these inline props to kit_base.rb, doesn't that mean we're adding these props to all Ruby kits?

Screenshot 2024-10-15 at 9 04 12 AM

Otherwise, looks really good to me! Clean, effective, and simple.

I've also got a few questions / comments which I've added.

nidaqg
nidaqg previously approved these changes Oct 15, 2024
@nidaqg nidaqg added the Code Approved Approved by a Playbook Admin label Oct 15, 2024
@jasperfurniss
Copy link
Contributor Author

@jasperfurniss

Since we're adding these inline props to kit_base.rb, doesn't that mean we're adding these props to all Ruby kits?

Screenshot 2024-10-15 at 9 04 12 AM

Otherwise, looks really good to me! Clean, effective, and simple.

I've also got a few questions / comments which I've added.

@kangaree We are only adding these to kits that use pb_content_tag. But none of it should be getting used/applied yet. Did your screen shot show up automatically in a doc example? Or did you implement code to make that happen.

@kangaree
Copy link
Contributor

kangaree commented Oct 15, 2024

@jasperfurniss
Since we're adding these inline props to kit_base.rb, doesn't that mean we're adding these props to all Ruby kits?
Screenshot 2024-10-15 at 9 04 12 AM
Otherwise, looks really good to me! Clean, effective, and simple.
I've also got a few questions / comments which I've added.

@kangaree We are only adding these to kits that use pb_content_tag. But none of it should be getting used/applied yet. Did your screen shot show up automatically in a doc example? Or did you implement code to make that happen.

@jasperfurniss I checked out this branch and in playbook/app/pb_kits/playbook/pb_fixed_confirmation_toast/docs/_fixed_confirmation_toast_default.html.erb, I made the following changes:

<%= pb_rails("fixed_confirmation_toast", props: {
  text: "Items Successfully Moved!!!",
  status: "success",
  height: "500px",
})%>

Edit: We talked in Connect and the plan moving forward we've got makes sense. This isn't applied to all ruby kits.

kangaree
kangaree previously approved these changes Oct 15, 2024
@jasperfurniss
Copy link
Contributor Author

<%= pb_rails("fixed_confirmation_toast", props: {
  text: "Items Successfully Moved!!!",
  status: "success",
  height: "500px",
})%>

@kangaree Ah! Perfect! Then yes, you're ahead of the game, and testing the "beta" stuff that we aren't advertising for use yet. This will likely be a 1 of 2 ...or 1 of 3 story setup for this work. Thanks for testing that out!

@jasperfurniss jasperfurniss added alpha and removed alpha labels Oct 15, 2024
Copy link

🎉 Congratulations on creating an Alpha Version!

Your Alpha for Ruby Gems is 14.5.0.pre.alpha.PLAY15814066

Your Alpha for NPM is 14.5.0-alpha.PLAY15814066

@jasperfurniss jasperfurniss added the enhancement New Features, Props, & Variants (USED IN CHANGELOG) label Oct 17, 2024
@nidaqg nidaqg added the Product Approved pending technical review, OK to merge to master label Oct 22, 2024
@nidaqg nidaqg added this pull request to the merge queue Oct 22, 2024
Merged via the queue into master with commit d7b8e5e Oct 22, 2024
5 checks passed
@nidaqg nidaqg deleted the PLAY-1581 branch October 22, 2024 19:38
github-merge-queue bot pushed a commit that referenced this pull request Oct 23, 2024
@jasperfurniss jasperfurniss restored the PLAY-1581 branch October 24, 2024 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
alpha Code Approved Approved by a Playbook Admin enhancement New Features, Props, & Variants (USED IN CHANGELOG) milano 20 MAX - Deploy this PR to a review environment via Milano Product Approved pending technical review, OK to merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants