-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Absolute positioned checkboxes overlay floated elements #26870
Conversation
We need to be careful around markdown rendering to stay compatible with GitHub's rendering. Could you post your sample markdown document here so we can compare this and GitHub's solution (assuming they have fixed 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.
For traceability, it looks likes this regression was introduced in #17395
I added a bit of a stress-test to compare and it looks like a marked improvement:
Here is some base text
- [ ] One
- [ ]
<img align="left" width="300" height="200" src="https://gitea.com/assets/img/logo.svg">
- [ ] Two
- [ ] Three
- [ ] Four
- [ ]
More text
#### dense list
- [ ] foo
- [ ] barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbar barbarbarbarbar
- [ ] baz
#### not so dense list
- [ ] foo
- [ ] barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbar barbarbarbarbar
- [ ] second item
- [ ] baz
web_src/css/markup/content.css
Outdated
@@ -144,7 +144,7 @@ | |||
|
|||
.markup ul, | |||
.markup ol { | |||
padding-left: 2em; | |||
padding-left: 0; |
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.
@rafh I'm worried about this style affecting other elements unrelated to checkboxes. Are there other markdown ul
ol
items this will apply to in markdown (I assume yes)?
Especially because this value was not changed here: https://github.com/go-gitea/gitea/pull/17411/files
(btw, this is an area front-end testing could validate)
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.
@kdumontnu I understand your point. Instead, we should change the em
back to 2em
and update the .task-list-item
to have margin-left: -2em;
. This will ensure that all checkboxes are aligned properly without impacting all ol
and ul
.
Here's an example of the markup and how it is rendered within Gitea.
- [x] example content
<img align="left" alt="" width="200" src="https://user-images.githubusercontent.com/6152817/265531274-d0b0b99a-db2b-4b6d-946d-277bfd24fc11.jpg">
- [x] example content
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur sed efficitur orci, eu cursus sapien. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur sed efficitur orci, eu cursus sapien. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur sed efficitur orci, eu cursus sapien. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur sed efficitur orci, eu cursus sapien. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur sed efficitur orci, eu cursus sapien.
<img style="clear: both" alt="" width="200" src="https://user-images.githubusercontent.com/6152817/265531274-d0b0b99a-db2b-4b6d-946d-277bfd24fc11.jpg">
- [ ] one
- [ ] two
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 seems like a reasonable solution to me, or maybe you can add a :not(.task-list-item)
to the selectors above and. Whatever will apply the style to the .task-list-item
items, but not change the styling for other lists.
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.
See comment
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.
Looks good - one minor thing below you might want to address is that the spacing between non-dense checkboxes is greater than spacing between non-dense list items (code below)
Here is some base text
- [ ] One
- [ ]
- Bullet
- Items
<img align="left" width="300" height="200" src="https://gitea.com/assets/img/logo.svg">
- [ ] Two
- [ ] Three
- [ ] Four
- [ ]
More text
#### dense list
- [ ] foo
- [ ] barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbar barbarbarbarbar
- [ ] baz
#### not so dense list
- [ ] foo
- [ ] barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbar barbarbarbarbar
- [ ] second item
- [ ] baz
<img align="left" width="300" height="200" src="https://gitea.com/assets/img/logo.svg">
- bullet
- items
Here is some base text
-
One
-
[ ]
-
Bullet
-
Items
- Two
- Three
- Four
- [ ]
More text
dense list
- foo
- barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbar barbarbarbarbar
- baz
not so dense list
-
foo
-
barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbarbar barbarbarbarbarbarbar barbarbarbarbar
-
second item
-
baz
- bullet
- items
@silverwind any other thoughts on this - seems like a clear improvement, and gets rid of the absolute positioning (which I'm sure is messing up some other rendering as well). |
This is how GitHub renders the following markdown and how Gitea renders (edited to show latest commit changes)
|
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 trust @kdumontnu's words that all potential regressions have been fixed.
Thanks, good catch! |
Can we as a first step copy GitHub's CSS exactly to reproduce their rendering, and then improve upon it by adding the margin-left to the checkboxes so they are not "glued" to the image? |
I've applied the styling used by GitHub. I added more padding to left-aligned images. Otherwise, our checkboxes would lie directly next to the images. Below I've consolidated all the markdown examples and added one more example at the top (empty checkboxes). I added this because the previous code had the following comment and I wanted to ensure this edge case was covered.
|
Currently, checkboxes are positioned as absolute. This positioning causes the input to overlay an element that has been floated within the editor. Floated elements are useful if you want your text to wrap around this element. This PR fixes the overlaying of checkboxes by removing the absolute positioning, updating the `ul` padding, and displaying`.task-list-item` `flex` to ensure inputs and the associated label are on the same line. Screenshots: Before: <img width="762" alt="Screenshot 2023-09-01 at 3 40 59 PM" src="https://github.com/go-gitea/gitea/assets/6152817/570247c7-7f5c-4697-bfc9-ad4655e37991"> After: <img width="762" alt="Screenshot 2023-09-01 at 3 42 20 PM" src="https://github.com/go-gitea/gitea/assets/6152817/db53df45-1294-4eee-84c0-b21ac4fdf805"> --------- Co-authored-by: rafh <rafaelheard@gmail.com>
) Backport #26870 by @rafh Currently, checkboxes are positioned as absolute. This positioning causes the input to overlay an element that has been floated within the editor. Floated elements are useful if you want your text to wrap around this element. This PR fixes the overlaying of checkboxes by removing the absolute positioning, updating the `ul` padding, and displaying`.task-list-item` `flex` to ensure inputs and the associated label are on the same line. Screenshots: Before: <img width="762" alt="Screenshot 2023-09-01 at 3 40 59 PM" src="https://github.com/go-gitea/gitea/assets/6152817/570247c7-7f5c-4697-bfc9-ad4655e37991"> After: <img width="762" alt="Screenshot 2023-09-01 at 3 42 20 PM" src="https://github.com/go-gitea/gitea/assets/6152817/db53df45-1294-4eee-84c0-b21ac4fdf805"> Co-authored-by: Rafael Heard <rafael.heard@gmail.com> Co-authored-by: rafh <rafaelheard@gmail.com>
Currently, checkboxes are positioned as absolute. This positioning causes the input to overlay an element that has been floated within the editor. Floated elements are useful if you want your text to wrap around this element. This PR fixes the overlaying of checkboxes by removing the absolute positioning, updating the
ul
padding, and displaying.task-list-item
flex
to ensure inputs and the associated label are on the same line.Screenshots:
Before:
After: