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

Setting value on a Select element triggers a React warning #117

Closed
nerfologist opened this issue Jul 4, 2017 · 2 comments
Closed

Setting value on a Select element triggers a React warning #117

nerfologist opened this issue Jul 4, 2017 · 2 comments

Comments

@nerfologist
Copy link

Issue summary

When creating a Select component, as soon as you specify a value property, the following warning is displayed in the console:

import React from 'react';
import ReactDOM from 'react-dom';
import { Select } from '@shopify/polaris';

const MyComp = () => <Select value=1 options={[1, 2]} />

ReactDOM.render(<MyComp/>, document.getElementById('root'));
Warning: Select elements must be either controlled or uncontrolled (specify
either the value prop, or the defaultValue prop, but not both). Decide between
using a controlled or uncontrolled select element and remove one of these props.
More info: https://fb.me/react-controlled-components

This happens because in the current implementation of <Select /> the defaultValue is always specified and set to '__placeholder__'.

Expected behavior

defaultValue is not set on the <select /> at the same time as value. No warning is raised by React.

Actual behavior

A warning is raised.

Steps to Reproduce the Problem

  1. Render a <Select /> component
  2. Add a value property
  3. See the warning appear in the console

Specifications

  • Polaris version: 1.1.1
  • React version: 15.6.1
  • Browser: Version 59.0.3071.115 (Official Build) (64-bit)
  • Device: MacBook Pro (Retina, 15-inch, Late 2013)
  • Operating System: macOS Sierra version 10.12.5 (16F73)

NOTE: This repo is only used for reporting issues and new feature requests. We are not accepting pull requests at this point in time.

@nerfologist
Copy link
Author

This is the same issue as #98, sorry for the duplicate issue (it was closed), but I believe this still needs fixing.

@lemonmade
Copy link
Member

Thanks for the issue, @nerfologist. I'm going to reopen the other issue since it has a few more comments/ context.

laurkim added a commit that referenced this issue Jun 8, 2023
### WHY are these changes introduced?

Resolves
[#117](Shopify/archive-polaris-backlog-2024#117).

### WHAT is this pull request doing?

Updates style for AccountConnection.

[Figma](https://www.figma.com/file/jLLkmo9r28hiqPvf4s90E4/Polaris-Uplift-Components-%5Bgen3%E2%80%93alpha%5D?type=design&node-id=26497%3A388&t=kkBificuKnaHXa5f-1)
    <details>
      <summary>AccountConnection — default screen size</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/a5e37730-5101-484c-b5b4-d61ed4ce4845"
alt="AccountConnection — default screen size">
    </details>
    <details>
      <summary>AccountConnection — sm breakpoint</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/f7987976-aca7-48b2-964e-019f1d936447"
alt="AccountConnection — sm breakpoint">
    </details>

### How to 🎩

[AccountConnection mega
story](https://5d559397bae39100201eedc1-sndslorqkw.chromatic.com/?path=/story/all-components-accountconnection--all)

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
juzser pushed a commit to juzser/polaris that referenced this issue Jul 27, 2023
### WHY are these changes introduced?

Resolves
[Shopify#117](https://github.com/Shopify/polaris-summer-editions/issues/117).

### WHAT is this pull request doing?

Updates style for AccountConnection.

[Figma](https://www.figma.com/file/jLLkmo9r28hiqPvf4s90E4/Polaris-Uplift-Components-%5Bgen3%E2%80%93alpha%5D?type=design&node-id=26497%3A388&t=kkBificuKnaHXa5f-1)
    <details>
      <summary>AccountConnection — default screen size</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/a5e37730-5101-484c-b5b4-d61ed4ce4845"
alt="AccountConnection — default screen size">
    </details>
    <details>
      <summary>AccountConnection — sm breakpoint</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/f7987976-aca7-48b2-964e-019f1d936447"
alt="AccountConnection — sm breakpoint">
    </details>

### How to 🎩

[AccountConnection mega
story](https://5d559397bae39100201eedc1-sndslorqkw.chromatic.com/?path=/story/all-components-accountconnection--all)

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
AnnaCheba pushed a commit to AnnaCheba/polaris that referenced this issue Apr 22, 2024
### WHY are these changes introduced?

Resolves
[Shopify#117](Shopify/archive-polaris-backlog-2024#117).

### WHAT is this pull request doing?

Updates style for AccountConnection.

[Figma](https://www.figma.com/file/jLLkmo9r28hiqPvf4s90E4/Polaris-Uplift-Components-%5Bgen3%E2%80%93alpha%5D?type=design&node-id=26497%3A388&t=kkBificuKnaHXa5f-1)
    <details>
      <summary>AccountConnection — default screen size</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/a5e37730-5101-484c-b5b4-d61ed4ce4845"
alt="AccountConnection — default screen size">
    </details>
    <details>
      <summary>AccountConnection — sm breakpoint</summary>
<img
src="https://github.com/Shopify/polaris/assets/26749317/f7987976-aca7-48b2-964e-019f1d936447"
alt="AccountConnection — sm breakpoint">
    </details>

### How to 🎩

[AccountConnection mega
story](https://5d559397bae39100201eedc1-sndslorqkw.chromatic.com/?path=/story/all-components-accountconnection--all)

🖥 [Local development
instructions](https://github.com/Shopify/polaris/blob/main/README.md#local-development)
🗒 [General tophatting
guidelines](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md)
📄 [Changelog
guidelines](https://github.com/Shopify/polaris/blob/main/.github/CONTRIBUTING.md#changelog)

### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [ ] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
- [ ] Updated the component's `README.md` with documentation changes
- [ ] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide
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

No branches or pull requests

2 participants