Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add Details/summary block #45055
Add Details/summary block #45055
Changes from all commits
be21b7f
9686338
ed02fa7
340fe0e
7468272
04b8f85
80375c0
84ede00
843ab63
3c2397f
8810fb0
82ce6b0
b56a2a7
26b07c4
14d6974
125b246
82643c4
10ffe31
c75a4aa
bc421cc
bd3474a
19cf689
a7c05f2
c31508c
4aef03e
0297f7d
a1698ee
98ee1ee
b740036
d72a333
95482f0
3e2cd78
d4b3923
767c4a2
8146396
89a07cf
cb2a3d8
7dc6665
460041a
c923448
ff578c2
52a404c
c9382af
64be240
492ad92
90f5249
731a8e8
eb47efc
597f6b6
772a334
2a3b19b
995102f
9edd68e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be another case to use the
View
component and theas
prop, rather than the raw HTML element. In addition to the prior reasoning, it could also help distinguish the block's root element from thesummary
value defined above it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation for View says it is only for divs.
I could use tagName, if that is easier to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm,
View as
, found an example!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not working. The
<details>
is not recognizing the<summary>
so it outputs the default text.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason,
"summary"
is being rendered as an HTML attribute e.g.as="summary"
on adiv
. My guess is there is a filter somewhere on what a validas
prop can be.If we did still want to avoid the summary variable being the same as the tag name we could do something like the snippet below. I don't think this is a critical improvement though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I encountered that when both the edit and save used View.
Personally I think using tagName would be clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the
<summary>
element be rendered by theRichText
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not been able to make this work because when the RichText is the
<summary>
, you can't addspace
to the text. Space is used to open and close the details. In the editor, this behavior is prevented withpreventDefault()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not seeing this style when testing in the frontend. Not sure why 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested on MacOS in Safari, Chrome, and Firefox, and I can see the pointer on the front 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's working for me now too!
It's strange, I'd even inspected the element and could see there was no
cursor
style.Oh well, nothing to worry about then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a small disparity between the editor and frontend when using border radius:
Editor
Frontend
It's something that can be fixed in a follow-up and shouldn't block merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am trying to reproduce this.
I am using Twenty Twenty-Three.
I do not see the clipping on macOS, in Firefox, Safari, or Chrome
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work now too:
Very strange. I tried switching between a few different themes and couldn't reproduce.
Maybe it was an issue with my local environment. Both this and the previous issue could have been due to styles not loading properly on the frontend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is a relief, thank you for double checking