[GH-ISSUE #148] Children of Picker/RadioButton are naively appended? #92

Closed
opened 2026-05-05 11:40:42 -06:00 by gitea-mirror · 1 comment
Owner

Originally created by @infiniteluke on GitHub (May 29, 2018).
Original GitHub issue: https://github.com/kusti8/proton-native/issues/148

Hi! Thanks for this great project. I really appreciate you making this oss. Very cool stuff.

I could be missing something here (I'm new to proton-native). I'm seeing strange behavior when a <Picker /> or <RadioButton />'s "Items" are driven by state. It appears their children, when rendered, are simply appended if they're new.

The if they're new part seems important but it's the part I understand the least. Wish I had more info here, but I don't. 🤷‍♂️

Here's an example app that reproduces the issue, by simply adding items to an array, then emptying it:
https://github.com/infiniteluke/proton-native-repro-append-issue

class Example extends Component {
  state = {
    things: []
  }
  add = (thing) => {
    this.setState({ things: [
      ...this.state.things,
      { id: `thing${this.state.things.length + 1}`, name: `Thing ${this.state.things.length + 1}` },
    ]})
  }
  remove = () => {
    this.setState({ things: [] });
  }
  render() { // all Components must have a render method
    return (
      <App> // you must always include App around everything
        <Window title="Proton Native Rocks!" size={{w: 300, h: 300}} menuBar={false}>
          <Box>
            <Text>{`${this.state.things.length} things added`}</Text>
            <Text>The RadioButton.Items and Picker.Items do not reset:</Text>
            <Picker>
              {this.state.things.map(thing => (
                <Picker.Item key={thing.id}>{thing.name}</Picker.Item>
              ))}
            </Picker>
            <RadioButtons>
              {this.state.things.map(thing => (
                <RadioButtons.Item key={thing.id}>{thing.name}</RadioButtons.Item>
              ))}
            </RadioButtons>
            <Button onClick={this.add}>Add</Button>
            <Button onClick={this.remove}>Remove</Button>
          </Box>
        </Window>
      </App>
    );
  }
}
screen shot 2018-05-28 at 8 30 52 pm screen shot 2018-05-28 at 9 26 56 pm

I would expect this to behave like react dom, smartly re-rendering the list using keys (https://reactjs.org/docs/reconciliation.html#recursing-on-children):
Here's a repo demonstrating a similar UI working correctly:
https://github.com/infiniteluke/proton-native-react-dom-comparison

class App extends Component {
  state = {
    things: []
  }
  add = (thing) => {
    this.setState({ things: [
      ...this.state.things,
      { id: `thing${this.state.things.length + 1}`, name: `Thing ${this.state.things.length + 1}` },
    ]})
  }
  remove = () => {  
    this.setState({ things: [] });
  }
  render() { // all Components must have a render method
    return (
      <div>
        <h1>{`${this.state.things.length} things added`}</h1>
        <h2>This works fine:</h2>
        <select>
          {this.state.things.map(thing => (
            <option key={thing.id}>{thing.name}</option>
          ))}
        </select>
        <form>
          {this.state.things.map(thing => (
            <div key={thing.id}>
              <input type="radio" id={thing.id} value={thing.name} />
              <label htmlFor={thing.id}>{thing.name}</label>
            </div>
          ))}
        </form>
        <button onClick={this.add}>Add</button>
        <button onClick={this.remove}>Remove</button>
      </div>
    );
  }
}
screen shot 2018-05-28 at 9 25 23 pm screen shot 2018-05-28 at 9 25 28 pm

I started looking around and I pinpointed https://github.com/kusti8/proton-native/blob/master/src/components/DesktopComponent.js#L151 which seems like it needs some heuristics around it for removing existing elements at least. If you point me in the right direction, I'd be happy to PR a fix.

Originally created by @infiniteluke on GitHub (May 29, 2018). Original GitHub issue: https://github.com/kusti8/proton-native/issues/148 Hi! Thanks for this great project. I really appreciate you making this oss. Very cool stuff. I could be missing something here (I'm new to proton-native). I'm seeing strange behavior when a `<Picker />` or `<RadioButton />`'s "Items" are driven by state. It appears their children, when rendered, are simply appended _if they're new_. The _if they're new_ part seems important but it's the part I understand the least. Wish I had more info here, but I don't. 🤷‍♂️ Here's an example app that reproduces the issue, by simply adding items to an array, then emptying it: https://github.com/infiniteluke/proton-native-repro-append-issue ```jsx class Example extends Component { state = { things: [] } add = (thing) => { this.setState({ things: [ ...this.state.things, { id: `thing${this.state.things.length + 1}`, name: `Thing ${this.state.things.length + 1}` }, ]}) } remove = () => { this.setState({ things: [] }); } render() { // all Components must have a render method return ( <App> // you must always include App around everything <Window title="Proton Native Rocks!" size={{w: 300, h: 300}} menuBar={false}> <Box> <Text>{`${this.state.things.length} things added`}</Text> <Text>The RadioButton.Items and Picker.Items do not reset:</Text> <Picker> {this.state.things.map(thing => ( <Picker.Item key={thing.id}>{thing.name}</Picker.Item> ))} </Picker> <RadioButtons> {this.state.things.map(thing => ( <RadioButtons.Item key={thing.id}>{thing.name}</RadioButtons.Item> ))} </RadioButtons> <Button onClick={this.add}>Add</Button> <Button onClick={this.remove}>Remove</Button> </Box> </Window> </App> ); } } ``` <img width="348" alt="screen shot 2018-05-28 at 8 30 52 pm" src="https://user-images.githubusercontent.com/1127238/40638028-8a94b968-62bd-11e8-934a-2906bacd100e.png"> <img width="316" alt="screen shot 2018-05-28 at 9 26 56 pm" src="https://user-images.githubusercontent.com/1127238/40638082-e5bf729c-62bd-11e8-9b20-973ec5d4a05d.png"> I would expect this to behave like react dom, smartly re-rendering the list using `keys` (https://reactjs.org/docs/reconciliation.html#recursing-on-children): Here's a repo demonstrating a similar UI working correctly: https://github.com/infiniteluke/proton-native-react-dom-comparison ```jsx class App extends Component { state = { things: [] } add = (thing) => { this.setState({ things: [ ...this.state.things, { id: `thing${this.state.things.length + 1}`, name: `Thing ${this.state.things.length + 1}` }, ]}) } remove = () => { this.setState({ things: [] }); } render() { // all Components must have a render method return ( <div> <h1>{`${this.state.things.length} things added`}</h1> <h2>This works fine:</h2> <select> {this.state.things.map(thing => ( <option key={thing.id}>{thing.name}</option> ))} </select> <form> {this.state.things.map(thing => ( <div key={thing.id}> <input type="radio" id={thing.id} value={thing.name} /> <label htmlFor={thing.id}>{thing.name}</label> </div> ))} </form> <button onClick={this.add}>Add</button> <button onClick={this.remove}>Remove</button> </div> ); } } ``` <img width="269" alt="screen shot 2018-05-28 at 9 25 23 pm" src="https://user-images.githubusercontent.com/1127238/40638043-a9eaf426-62bd-11e8-9805-bc2de457ba41.png"> <img width="257" alt="screen shot 2018-05-28 at 9 25 28 pm" src="https://user-images.githubusercontent.com/1127238/40638050-aed5452c-62bd-11e8-93cf-5df95df84673.png"> I started looking around and I pinpointed https://github.com/kusti8/proton-native/blob/master/src/components/DesktopComponent.js#L151 which seems like it needs some heuristics around it for removing existing elements at least. If you point me in the right direction, I'd be happy to PR a fix.
Author
Owner

@mischnic commented on GitHub (May 29, 2018):

In libui there's only uiRadioButtonsAppend and uiComboboxAppend (issue: https://github.com/andlabs/libui/issues/309).
So any fix needs (for now) to use a workaround like UiSlider does (remove and re-add the component).

<!-- gh-comment-id:392696699 --> @mischnic commented on GitHub (May 29, 2018): In libui there's only [`uiRadioButtonsAppend`](https://github.com/andlabs/libui/blob/c3be9f221c236c594c80713b12daa9dd818aeacb/ui.h#L245) and [`uiComboboxAppend`](https://github.com/andlabs/libui/blob/c3be9f221c236c594c80713b12daa9dd818aeacb/ui.h#L228) (issue: https://github.com/andlabs/libui/issues/309). So any fix needs (for now) to use a workaround like UiSlider [does](https://github.com/kusti8/proton-native/blob/master/src/components/Slider.js#L20) (remove and re-add the component).
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/proton-native#92
No description provided.