Skip to content

Upgrade babel#4908

Closed
cfarm wants to merge 2 commits intomasterfrom
upgrade_babel
Closed

Upgrade babel#4908
cfarm wants to merge 2 commits intomasterfrom
upgrade_babel

Conversation

@cfarm
Copy link
Copy Markdown
Contributor

@cfarm cfarm commented Mar 25, 2019

Trying to sort out this branch referenced in cfpb/cfpb-chart-builder#162

Additions

Removals

Changes

Testing

Screenshots

Notes

Todos

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the CFPB development guidelines
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Browsers

  • Chrome on desktop
  • Firefox
  • Safari on macOS
  • Edge
  • Internet Explorer 9, 10, and 11
  • Safari on iOS
  • Chrome on Android

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

@anselmbradford anselmbradford marked this pull request as ready for review April 12, 2019 11:56
accesslint[bot]
accesslint bot suggested changes Apr 12, 2019
View reviewed changes
Copy link
Copy Markdown

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

test/unit_tests/js/organisms/FormSubmit-spec.js
Protection Bureau to process your request or
inquiry.&nbsp;<a class="" href="/privacy/privacy-policy/">See more</a>.
</p>
<input class="a-btn a-btn__full-on-xs" type="submit" value="Sign up">
Copy link
Copy Markdown

@accesslint accesslint bot Apr 12, 2019

Choose a reason for hiding this comment

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

Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.

test/unit_tests/js/organisms/FormSubmit-spec.js
</div>

<div class="form-group">
<input type="hidden" name="code">
Copy link
Copy Markdown

@accesslint accesslint bot Apr 12, 2019

Choose a reason for hiding this comment

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

Looks like there's a label missing for this input. That makes it hard for people using screen readers or voice control to use the input. If you don't want a visual label, try an aria-label attribute.

@anselmbradford
Copy link
Copy Markdown
Member

anselmbradford commented Apr 12, 2019

These changes were already merged on #4635
Likely the description in cfpb/cfpb-chart-builder#162 was written prior to it being merged.

@anselmbradford anselmbradford deleted the upgrade_babel branch April 12, 2019 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@accesslint accesslint[bot] accesslint[bot] requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cfarm @anselmbradford