Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add form as a prop for button kit #1801

Merged
merged 4 commits into from
Mar 1, 2022
Merged

Add form as a prop for button kit #1801

merged 4 commits into from
Mar 1, 2022

Conversation

nehaabraham
Copy link
Contributor

@nehaabraham nehaabraham commented Feb 28, 2022

Allows forms to be connected to a button that is created outside of the form.

Screens

[INSERT SCREENSHOT]

Breaking Changes

[Yes/No (Explain)]

Runway Ticket URL

[INSERT URL]

How to test this

[INSERT TESTING DETAILS]

Checklist:

  • LABELS Add a label: enhancement, bug, improvement, new kit, deprecated, or breaking. See Changelog & Labels for details.
  • DEPLOY Please add the Milano label when you are ready for a review.
  • SCREENSHOT Please add a screen shot or two.
  • SPECS Please cover your changes with specs.
  • READ DOCS Please make sure you have read and understand the Playbook Release Process

Allows forms to be connected to a button that is created outside of the form
@nehaabraham nehaabraham requested a review from a team as a code owner February 28, 2022 17:30
@nehaabraham nehaabraham self-assigned this Feb 28, 2022
@nehaabraham nehaabraham added enhancement New Features, Props, & Variants (USED IN CHANGELOG) milano 20 MAX - Deploy this PR to a review environment via Milano Needs Review labels Feb 28, 2022
@app-milano app-milano bot temporarily deployed to pr1801 February 28, 2022 19:34 Inactive
@Beznus
Copy link
Contributor

Beznus commented Feb 28, 2022

Screen Shot 2022-02-28 at 4 30 18 PM
Screen Shot 2022-02-28 at 4 30 34 PM

@@ -15,6 +15,7 @@ type ButtonPropTypes = {
data?: object,
disabled?: boolean,
fixedWidth?: boolean,
form?: string,
Copy link
Contributor

Choose a reason for hiding this comment

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

@nehaabraham I made this optional since there will be many instances in Nitro which will not be sending this prop. Once this kit is converted to Typescript, not having the optional ? will throw errors.

it "form", :aggregate_failures do
expect(subject.new(form: "form-id").options).to include(:form)
expect(subject.new.options).to_not include(:form)
end
Copy link
Contributor

Choose a reason for hiding this comment

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

@nehaabraham I also added a test to ensure that form prop is present when sent.

@thestephenmarshall thestephenmarshall added Ready for Release merged to master, ready for a versioned released and removed milano 20 MAX - Deploy this PR to a review environment via Milano Needs Review labels Mar 1, 2022
@thestephenmarshall thestephenmarshall changed the title Add form as a prop for buttons. Add form as a prop for button kit Mar 1, 2022
@thestephenmarshall thestephenmarshall merged commit 2faef2f into master Mar 1, 2022
@thestephenmarshall thestephenmarshall deleted the add-form-prop branch March 1, 2022 19:46
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New Features, Props, & Variants (USED IN CHANGELOG) Ready for Release merged to master, ready for a versioned released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants