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

Clean up some workarounds for old upstream bugs #40199

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/files/renovate-post-upgrade.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ function die {

# Renovate has a bug where they modify `.npmrc` and don't clean up after themselves,
# resulting in those modifications being included in the diff.
# https://github.com/renovatebot/renovate/issues/23528
# Further, it seems they're reluctant to even admit this is actually a bug, and would
# rather cast aspersions than collaborate on a fix.
# https://github.com/renovatebot/renovate/discussions/23489
# So work around it by manually reverting the file.
git restore .npmrc

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: https://github.com/storybookjs/storybook/issues/7215 was fixed a while back, remove TODOs. And fix a wrong prop in one story.


Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,16 @@ import ActionButton from '../index.jsx';
export default {
title: 'JS Packages/Components/Action Button',
component: ActionButton,
// TODO: Storybook Actions are not working. See https://github.com/storybookjs/storybook/issues/7215
argTypes: {
onButtonClick: { action: 'clicked' },
onClick: { action: 'clicked' },
},
};

// Export additional stories using pre-defined values
const Template = args => <ActionButton { ...args } />;

const DefaultArgs = {
onButtonClick: action( 'onButtonClick' ),
onClick: action( 'onButtonClick' ),
displayError: false,
isLoading: false,
label: 'Action!',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type { StoryFn, Meta } from '@storybook/react';
export default {
title: 'JS Packages/Components/Pricing Card',
component: PricingCard,
// TODO: Storybook Actions are not working. See https://github.com/storybookjs/storybook/issues/7215
argTypes: {
onCtaClick: { action: 'clicked' },
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Add missing ids to radio buttons in the confirmation form.
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,15 @@ export function ConfirmationForm( { keyringResult, onComplete, isAdmin }: Confir
: index === 0;

return (
// eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869
<label key={ option.value } className={ styles[ 'account-label' ] } aria-required>
<label
key={ option.value }
htmlFor={ `external_user_ID__${ option.value }` }
className={ styles[ 'account-label' ] }
aria-required
>
<input
type="radio"
id={ `external_user_ID__${ option.value }` }
name="external_user_ID"
value={ option.value }
defaultChecked={ defaultChecked }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Update example with ids for jsx-a11y/label-has-associated-control.
22 changes: 16 additions & 6 deletions projects/js-packages/social-logos/src/react/example.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,27 @@ function SocialLogosExample() {
<div className="display-control-group">
<div className="display-control">
<h4>Small icons</h4>
{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
<label className="switch">
<input type="checkbox" onChange={ handleSmallIconsToggle } checked={ useSmallIcons } />
{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/578 */ }
<label className="switch" htmlFor="useSmallIcons">
<input
id="useSmallIcons"
type="checkbox"
onChange={ handleSmallIconsToggle }
checked={ useSmallIcons }
/>
<span className="handle"></span>
</label>
</div>
<div className="display-control">
<h4>Icon names</h4>
{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
<label className="switch">
<input type="checkbox" onChange={ handleIconNamesToggle } checked={ showIconNames } />
{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/578 */ }
<label className="switch" htmlFor="showIconNames">
<input
id="showIconNames"
type="checkbox"
onChange={ handleIconNamesToggle }
checked={ showIconNames }
/>
<span className="handle"></span>
<span className="switch-label" data-on="On" data-off="Off"></span>
</label>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: changed
Comment: Move reference to a Storybook bug to the code specifically for the workaround.


Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Update Storybook FAQ reference.
5 changes: 2 additions & 3 deletions projects/js-packages/storybook/storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ const sbconfig = {
'storybook-addon-mock',
'@storybook/addon-webpack5-compiler-babel',
],
// Workaround:
// https://github.com/storybookjs/storybook/issues/12270
webpackFinal: async config => {
// Remove source maps in production builds.
if ( process.env.NODE_ENV === 'production' ) {
Expand All @@ -63,6 +61,7 @@ const sbconfig = {
// Find the DefinePlugin
const plugin = config.plugins.find( p => p.definitions?.[ 'process.env' ] );
// Add custom env variables
// https://github.com/storybookjs/storybook/issues/12270
Object.keys( customEnvVariables ).forEach( key => {
plugin.definitions[ 'process.env' ][ key ] = JSON.stringify( customEnvVariables[ key ] );
} );
Expand Down Expand Up @@ -127,7 +126,7 @@ const sbconfig = {
},
framework: {
// Workaround https://github.com/storybookjs/storybook/issues/21710
// from https://storybook.js.org/docs/react/faq#how-do-i-fix-module-resolution-while-using-pnpm-plug-n-play
// from https://storybook.js.org/docs/faq#how-do-i-fix-module-resolution-in-special-environments
name: path.dirname( require.resolve( '@storybook/react-webpack5/package.json' ) ),
options: {},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Significance: patch
Type: fixed
Comment: Remove workaround for fixed WPCS bug.


3 changes: 1 addition & 2 deletions projects/packages/forms/src/contact-form/class-admin.php
Original file line number Diff line number Diff line change
Expand Up @@ -1411,8 +1411,7 @@ public function grunion_recheck_queue() {
$query = 'post_type=feedback&post_status=publish';

if ( isset( $_POST['limit'] ) && isset( $_POST['offset'] ) ) {
// phpcs:ignore Generic.Strings.UnnecessaryStringConcat.Found -- Avoiding https://github.com/WordPress/WordPress-Coding-Standards/issues/2390
$query .= '&posts_per' . '_page=' . (int) $_POST['limit'] . '&offset=' . (int) $_POST['offset'];
$query .= '&posts_per_page=' . (int) $_POST['limit'] . '&offset=' . (int) $_POST['offset'];
}

$approved_feedbacks = get_posts( $query );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Add missing ids in Verbum EmailForm.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Explicitly set `htmlFor` in recommended tags modal FormLabel.
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ export const EmailForm = ( { shouldShowEmailForm }: EmailFormProps ) => {
{ shouldShowEmailForm && (
<div className="verbum-form__wrapper">
<div className="verbum-form__content">
{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
<label className="verbum__label">
<label htmlFor="verbum-email-form-email" className="verbum__label">
<Email />
<input
id="verbum-email-form-email"
className={ clsx( 'verbum-form__email', {
'invalid-form-data': isValidEmail.value === false && isEmailTouched.value,
} ) }
Expand All @@ -108,10 +108,10 @@ export const EmailForm = ( { shouldShowEmailForm }: EmailFormProps ) => {
/>
</label>

{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
<label className="verbum__label">
<label htmlFor="verbum-email-form-name" className="verbum__label">
<Name />
<input
id="verbum-email-form-name"
className={ clsx( 'verbum-form__name', {
'invalid-form-data': isValidAuthor.value === false && isNameTouched.value,
} ) }
Expand All @@ -130,10 +130,10 @@ export const EmailForm = ( { shouldShowEmailForm }: EmailFormProps ) => {
/>
</label>

{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control -- https://github.com/jsx-eslint/eslint-plugin-jsx-a11y/issues/869 */ }
<label className="verbum__label">
<label htmlFor="verbum-email-form-website" className="verbum__label">
<Website />
<input
id="verbum-email-form-website"
className="verbum-form__website"
type="text"
spellCheck={ false }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ const FormLabel: FunctionComponent< Props & LabelProps > = ( {
required,
optional,
className, // Via LabelProps
htmlFor,
...labelProps
} ) => {
const hasChildren: boolean = Children.count( children ) > 0;

return (
// eslint-disable-next-line jsx-a11y/label-has-associated-control
<label { ...labelProps } className={ clsx( className, 'form-label' ) }>
<label htmlFor={ htmlFor } { ...labelProps } className={ clsx( className, 'form-label' ) }>
{ children }
{ hasChildren && required && (
<small className="form-label__required">{ __( 'Required', 'jetpack-mu-wpcom' ) }</small>
Expand Down
2 changes: 0 additions & 2 deletions tools/js-tools/jest/jest-resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// List them here and the resolver will adjust the conditions to resolve them as "node" instead.
// cf. https://github.com/microsoft/accessibility-insights-web/pull/5421#issuecomment-1109168149
const badBrowserPackages = new Set( [
// v3 is still supposed to be commonjs-compatible. https://github.com/ai/nanoid/issues/462
'nanoid',
// https://github.com/LeaVerou/parsel/issues/79
'parsel-js',
] );
Expand Down
Loading