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
<details> open attribute not synchronised #15486
Comments
It looks like it does synchronizes with the virtual dom. Also, you don't need to do |
Thanks @baruchvlz. So is the official advice that |
I just ran into the same issue today, and I fixed it by adding an const onToggle = event => {
event.preventDefault();
setOpen(!open);
};
// Later on...
return (
<div className="App">
<details open={open} onClick={onToggle}>...</details>
</div>
) |
Awesome @alejandronanez I'll go with that for now. |
@alejandronanez is right!details element have Default behavior! So,it is not controlled by onclickEvent |
Also the |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you! |
This "stale" bot thing is painful. It shouldn't close bugs without someone checks it doesn't happen anymore... |
I do not have a solution for Summary Explanation Example SummaryContent (clicking here has no effect) With React: Reference
Browser Support |
To mimic the default behavior I did the following:
|
The title of this issue is inaccurate: React does keep the It's confusing because, unlike most other native DOM elements, the
This is why the element does not open the first time you click. Because this happens instantly, it's not obvious that the state is actually being toggled twice. When you click on the element again, this happens:
This is why you can toggle the element after the first click, even though React and the DOM disagree about the current state. Every time it re-renders, React sees that the DOM is already in the state that it wants to be in, so it does not update the DOM and behaves as if the As mentioned in #15486 (comment), the solution is to call #15486 (comment) mentioned that there is a tl;dr This is a confusing case, but is probably not an issue with React. The solution is to call |
Finally, as the above comments found a solution: to use event.preventDefault() inside an onClick on the summary element, here is the code. import { useState } from "react";
export default function Details({ summary, children, startOpen = false }){
const [open, setOpen] = useState(startOpen);
return (
<details {...(open ? { open: true } : {})}>
<summary
onClick={(e) => {
e.preventDefault();
setOpen((open) => !open);
}}
>
{typeof summary === "function" ? summary(open) : summary}
</summary>
{open && children}
</details>
);
}; Usage: function App() {
return (
<div className="App">
<Details summary={(open) => `${open ? "View" : "Hide"} contents`}>
<p>The summary prop can be a function which gets the open value</p>
</Details>
<hr/>
<Details summary="Details" startOpen={true}>
<p>
These contents are added/removed from the DOM depending on whether
details is open or not.
</p>
</Details>
<hr/>
</div>
);
} See working / edit the code: https://codesandbox.io/s/beautiful-borg-nbh93?file=/src/App.js |
Hmm... this wasn't an issue for me in Chrome. Which browser did you notice this behavior? |
I just stumbled a somewhat similar bug, that I think is related to this. I have a |
The explanation was hidden in a large comment, but I mentioned this case above: #15486 (comment)
It seems like my sandbox did not include the button that I mentioned in the comment, but I updated it now to add the button. In your sandbox, there is no button for toggling the React state alone, but because |
Oh, I see. So this is how it works 🤔 Thanks for the explanation. I'm gonna go with the solution proposed above, using a click event listener and |
Was going to make <details> controlled but apparently you can't, facebook/react#15486
The idea that breaking an html element isn't a bug, is a problem. If a native element with w3c settled behavior is broken by React, React is broken, and this is a legit bug. |
As far as I can tell nobody in this thread has suggested that this is not a problem. Maybe you were confused by my earlier comment where I said this?:
I simply meant that the specific claim that the title made is technically inaccurate. I do think this is a very confusing thing that will waste a lot of people's time if it is not somehow fixed (or at least better documented). In case you missed it, it is still possible to use the |
It's possible to react to the native import { type ReactElement, useState } from "react";
function Component(): ReactElement {
const [open, setOpen] = useState(false);
const handleToggle = (
event: { currentTarget: { open: boolean } },
) => setOpen(event.currentTarget.open);
return (
<details onToggle={handleToggle} open={open}>
<summary>{open ? "Hide" : "Show"}</summary>
<p>More content</p>
</details>
);
} |
Using
|
@butchler That's only an issue when using a function that inverts the state value (like in your CodeSandbox example) — that's what causes the race condition and infinite re-render scenario. When setting it according to the value in the dispatched event (like in the code shown in my previous comment), you can be confident that won't happen. If the new boolean value is opposite of the current value, it will re-render once. If it's the same, no new render will occur. From the React docs:
|
Correct, and that's what I meant by "if the state is updated by anything other than the event handler". Sorry if my phrasing wasn't clear. This means that for the case where you only need to know if the For that reason, personally I wouldn't recommend |
If you do use |
@butchler You can have confidence that won't be an issue either — it'll still work! Below I've included a reproducible example that you can use to verify. No build tools are needed — just save the contents as a local file and open in a browser. In case you don't have a text editor available, here's a link to a hosted version: https://jsfiddle.net/4w7tuLfk/
|
I apologize, I didn't notice the difference between your suggestion and the previous suggestions in this thread about using the In hindsight it seems so obvious that I don't know why I never thought of it myself. 😅 |
I noticed that the new React docs have pages documenting how to use common types of controlled elements such as If someone added a similar page describing usage of the |
Note that browsers can open Here's a demo: https://jsfiddle.net/SheepTester/57chzoen/ |
What is the current behavior?
open
attribute does not synchronise with the virtual dom on thedetails
element, when using thedetails
element as a controlled component.I have a codesandbox here that demonstrates the current behaviour.
What is the expected behavior?
The
open
attribute stays synchronised with the virtual dom and is added/removed.Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
React:
16.8.6
Browser: Chrome
Did this work in previous versions of React? No
The text was updated successfully, but these errors were encountered: