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

feat(core): internals controller improvements #2296

Merged
merged 12 commits into from Jan 5, 2023

Conversation

bennypowers
Copy link
Member

@bennypowers bennypowers commented Jan 3, 2023

What I did

  • proxy role and aria* props in ElementInternals to InternalsController
  • wrap the host's form*Callback methods in InternalsController
  • add formDisabled: boolean to InternalsController
  • disable BaseButton if it's disabled either by attr or by form, without requiring the reflected attr when form is disabled.

Testing Instructions

Check out button's form controls demo and make sure everything's kosher there

@changeset-bot
Copy link

changeset-bot bot commented Jan 3, 2023

🦋 Changeset detected

Latest commit: e378556

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

This PR includes changesets to release 3 packages
Name Type
@patternfly/pfe-button Patch
@patternfly/pfe-core Minor
@patternfly/pfe-tools 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

@bennypowers bennypowers changed the title Feat/core/internals controller improvements feat(core): internals controller improvements Jan 3, 2023
@github-actions github-actions bot added demo Updating demo pages functionality Functionality, typically pertaining to the JavaScript. labels Jan 3, 2023
@github-actions github-actions bot added this to In progress in Workflow Jan 3, 2023
@bennypowers bennypowers force-pushed the feat/core/internals-controller-improvements branch from 853b619 to 9da29e8 Compare January 3, 2023 13:31
@bennypowers bennypowers enabled auto-merge (squash) January 3, 2023 13:32
@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2023

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit 7981bf2
😎 Deploy Preview https://deploy-preview-2296--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added the AT passed Automated testing has passed label Jan 3, 2023
@bennypowers bennypowers force-pushed the feat/core/internals-controller-improvements branch 2 times, most recently from 17e7e93 to 5fe3ff7 Compare January 3, 2023 17:10
@bennypowers bennypowers force-pushed the feat/core/internals-controller-improvements branch from 9b968bb to 28176a6 Compare January 4, 2023 07:39
@bennypowers
Copy link
Member Author

when the fieldset is disabled but the button is not disabled, user is able to tab to the button, although it renders in grey. thanks for the report, @brianferry

@bennypowers
Copy link
Member Author

I toyed around with some ways to hook into the form callbacks from the controller, but none were particularly satisfying, so let's do all that on the button class instead

@github-actions github-actions bot added tests Related to testing tools Development and build tools labels Jan 5, 2023
Copy link
Contributor

@heyMP heyMP left a comment

Choose a reason for hiding this comment

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

👍

Workflow automation moved this from In progress to Approved Jan 5, 2023
@bennypowers bennypowers merged commit 0fe6c52 into main Jan 5, 2023
Workflow automation moved this from Approved to Done Jan 5, 2023
@bennypowers bennypowers deleted the feat/core/internals-controller-improvements branch January 5, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AT passed Automated testing has passed demo Updating demo pages functionality Functionality, typically pertaining to the JavaScript. ready to merge tests Related to testing tools Development and build tools
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants