FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(login page): removes ariaLabel from checkbox by jenny-s51 · Pull Request #2455 · patternfly/patternfly-react · GitHub

/ patternfly-react Public

fix(login page): removes ariaLabel from checkbox - #2455

Merged
tlabaj merged 5 commits into
patternfly:masterfrom
jenny-s51:iss1737
Jul 11, 2019
ConversationCommits (5)ChecksFiles changed
Merged

fix(login page): removes ariaLabel from checkbox#2455
tlabaj merged 5 commits into
patternfly:masterfrom
jenny-s51:iss1737

Conversation

jenny-s51 commented Jul 3, 2019

Copy link
Copy Markdown
Contributor

closes #1737

jenny-s51 requested review from dlabaj, jgiardino and tlabaj July 3, 2019 17:23

patternfly-build commented Jul 3, 2019

Copy link
Copy Markdown
Collaborator

PatternFly-React preview: https://patternfly-react-pr-2455.surge.sh

jgiardino commented Jul 3, 2019

Copy link
Copy Markdown
Contributor

These updates look good! The only question I have is whether this would be a breaking change (and if so, are there any changes we can make now without waiting for a breaking change release)? I'm not able to test what happens if rememberMeAriaLabel is defined on <LoginForm> (for some reason, the code editor section is read-only).

jenny-s51 commented Jul 3, 2019

Copy link
Copy Markdown
Contributor Author

@jgiardino thank you for your feedback! I'm curious as well -- @tlabaj would this be a breaking change?

jenny-s51 self-assigned this Jul 3, 2019
jenny-s51 added Breaking change 💥 this change requires a major release and has API changes. Do Not Merge labels Jul 3, 2019
dgutride reviewed Jul 5, 2019
View reviewed changes
Comment thread packages/patternfly-4/react-core/src/components/LoginPage/LoginForm.tsx
label={rememberMeLabel}
checked={isRememberMeChecked}
onChange={onChangeRememberMe}
aria-label={rememberMeAriaLabel || rememberMeLabel}

dgutride Jul 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This looks good to me - let's add one more comment up above in rememberMeAriaLabel (to the comment section noting this propType is now deprecated and is no longer used in the code. Also, please open an issue with a breaking change label indicating we should delete this propType in a future breaking change release.

jenny-s51 Jul 5, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Just pushed again with this addition to the comment. Should be all set now -- made a new breaking change issue as well.

jenny-s51 removed Do Not Merge Breaking change 💥 this change requires a major release and has API changes. labels Jul 5, 2019

tlabaj commented Jul 5, 2019

Copy link
Copy Markdown
Contributor

Can you mention new issue here please? Thnaks.

jenny-s51 commented Jul 5, 2019

Copy link
Copy Markdown
Contributor Author

@tlabaj It's #2464 🙂

jenny-s51 requested a review from dgutride July 8, 2019 13:06
tlabaj assigned jessiehuff and unassigned jenny-s51 Jul 10, 2019
tlabaj approved these changes Jul 11, 2019
View reviewed changes

tlabaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

tlabaj merged commit 3cf57e3 into patternfly:master Jul 11, 2019

patternfly-build commented Jul 11, 2019

Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • @patternfly/react-core@3.70.1
  • @patternfly/react-docs@4.8.74
  • @patternfly/react-inline-edit-extension@2.9.37
  • demo-app-ts@2.11.1
  • @patternfly/react-integration@2.11.1
  • @patternfly/react-table@2.14.11
  • @patternfly/react-topology@2.6.8
  • @patternfly/react-virtualized-extension@1.1.70

Thanks for your contribution! 🎉

jenny-s51 mentioned this pull request Jul 15, 2019
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

dlabaj Awaiting requested review from dlabaj
jgiardino Awaiting requested review from jgiardino
2 more reviewers
dgutride dgutride approved these changes
tlabaj tlabaj approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

jessiehuff

Labels

A11y

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

LoginPage - remove aria-label from checkbox

6 participants

Back | FazBrowse Home | New Git URL