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

[TextField] Remove redundant null check #2783

Merged
merged 1 commit into from
Mar 13, 2020

Conversation

sambostock
Copy link
Contributor

@sambostock sambostock commented Feb 27, 2020

WHY are these changes introduced?

There is a redundant null check in <TextField />'s normalizeAutocomplete function. It should already be covered by

else {
  return autocomplete;
}

WHAT is this pull request doing?

The redundant condition is removed.

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Feb 27, 2020

🟡 This pull request modifies 2 files and might impact 9 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 9)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/TextField/TextField.tsx (total: 9)

Files potentially affected (total: 9)

@sambostock sambostock force-pushed the remove-redundant-condition branch from 208d23e to ec0b0f9 Compare February 27, 2020 00:41
@sambostock sambostock requested a review from chloerice March 5, 2020 20:18
Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

Tophat works as expected :shipit:

Click to view collapsed Playground code
import React, {useState, useCallback} from 'react';
import {Page, TextField} from '../src';

interface TextFieldProps {
  autocomplete?: boolean | string;
}

function TextFieldExample({autocomplete}: TextFieldProps) {
  const [value, setValue] = useState('Jaded Pixel');
  const handleChange = useCallback((newValue) => setValue(newValue), []);

  return (
    <TextField
      autoComplete={autocomplete}
      label="Store name"
      value={value}
      onChange={handleChange}
    />
  );
}

export function Playground() {
  return (
    <Page title="Playground">
      <TextFieldExample autocomplete />
      <TextFieldExample />
      <TextFieldExample autocomplete={undefined} />
      <TextFieldExample autocomplete="off" />
    </Page>
  );
}

This check is redundant, since it should already be covered by

    else {
      return autoComplete;
    }
@sambostock sambostock force-pushed the remove-redundant-condition branch from ec0b0f9 to 5a0a6da Compare March 11, 2020 17:58
@sambostock sambostock merged commit 8dc950f into master Mar 13, 2020
@sambostock sambostock deleted the remove-redundant-condition branch March 13, 2020 16:48
@tmlayton tmlayton temporarily deployed to production March 14, 2020 00:47 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants