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

Fix task-list checkbox styling #10668

Merged
merged 3 commits into from
Mar 8, 2020
Merged

Conversation

cipherboy
Copy link
Contributor

This pull request does two things:

#9656 strictly describes the missing space. However, commenters asked for the bullet to be removed as well. This is a little more tricky as the GFM spec doesn't mention adding a class to the <ul> (the way pandoc does). It might be possible to convince the goldmark maintainer to add that class though, I'm not sure.

An alternative is to wait until CSS4 selectors are available. This should include the :has() selector, which would allow us to select based on containing an input field. Something like li:has(input[type="checkbox"]). But caniuse says I can't use it because nobody supports it.


When rendered with Pandoc, it now looks like:
Screenshot from 2020-03-08 10-22-36


When rendered with Goldmark, it now looks like:
Screenshot from 2020-03-08 10-25-07


Note: I'm using go v1.13.6 and it looks like the go.sum file didn't change correctly. What version of go should I be using to match this project's go.sum file preferences?

The pandoc renderer will append the class "task-list" to the ul element
wrapping a li with one or more check-boxes. This allows us to select for
them, removing their list-style-type property. However, goldmark and the
gfm spec doesn't specify the "task-list" class name, so we can't use
that to fix the issue there.

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
This version adds the missing space after a checkbox.

Resolves: go-gitea#9656

Signed-off-by: Alexander Scheel <alexander.m.scheel@gmail.com>
@zeripath zeripath added the topic/ui Change the appearance of the Gitea UI label Mar 8, 2020
@zeripath zeripath added this to the 1.12.0 milestone Mar 8, 2020
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 8, 2020
@silverwind
Copy link
Member

What version of go should I be using to match this project's go.sum file preferences

Latest go 1.13 should be fine. We're not on 1.14 yet.

This is LGTM, besides the odd checkbox rendering in those screenshots, but it's not related to this PR.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 8, 2020
@silverwind
Copy link
Member

Tried the PR. It does not seem to add the class for me:

image

image

@silverwind
Copy link
Member

Nevermind, it does work with pandoc enabled, e.g.

[markup.markdown]
ENABLED         = true
FILE_EXTENSIONS = .md,.markdown
RENDER_COMMAND  = pandoc -f markdown -t html --katex

@cipherboy
Copy link
Contributor Author

This is LGTM, besides the odd checkbox rendering in those screenshots, but it's not related to this PR.

Sorry, what's wrong with the checkboxes? If/when we get a class on the <ul> element or CSS4 selectors, we could fix the indentation to be consistent with GitHub (where a task list isn't indented and is more left-justified, unless it is nested obviously).

But the checkboxes themselves are correct. I'm on Fedora 31, under GNOME, and they match the system theme:

Screenshot from 2020-03-08 14-44-19

Tried the PR. It does not seem to add the class for me:

What are your rendering settings? As I mentioned above ("Styles task-lists to remove the bullets, when rendered with pandoc."), you have to use pandoc. My config is:

[markup.sanitizer]
ELEMENT    = span
ALLOW_ATTR = class
REGEXP     = ^(math\s*|inline\s*|display\s*){0,3}$

[markup.markdown]
ENABLED         = true
FILE_EXTENSIONS = .md,.markdown
RENDER_COMMAND  = bash /home/git/custom/conf/pandoc.sh
$ cat pandoc.sh
#!/bin/bash

export PANDOC_GITEA_PREFIX_SRC="$GITEA_PREFIX_SRC"
export PANDOC_GITEA_PREFIX_RAW="$GITEA_PREFIX_RAW"

pandoc -f markdown -t html -M PANDOC_GITEA_PREFIX_SRC="$GITEA_PREFIX_SRC" -M PANDOC_GITEA_PREFIX_RAW="$GITEA_PREFIX_RAW" --lua-filter=/home/git/custom/conf/pandoc.lua --katex

@silverwind
Copy link
Member

GNOME

Ah, that's why. They just look a bit "unusual" to me 😉

Thanks for the pandoc info, I just switched to it.

@silverwind
Copy link
Member

So to get this to work with the bluemonday sanitizer too, we need goldmark to add the class, right? Would you mind opening a goldmark issue on it?

@codecov-io
Copy link

Codecov Report

Merging #10668 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10668      +/-   ##
==========================================
+ Coverage   43.61%   43.61%   +<.01%     
==========================================
  Files         588      588              
  Lines       82487    82490       +3     
==========================================
+ Hits        35973    35979       +6     
+ Misses      42053    42052       -1     
+ Partials     4461     4459       -2
Impacted Files Coverage Δ
modules/markup/sanitizer.go 92.5% <100%> (+0.29%) ⬆️
modules/indexer/stats/db.go 50% <0%> (-9.38%) ⬇️
modules/queue/workerpool.go 55.87% <0%> (-2.14%) ⬇️
modules/log/event.go 64.61% <0%> (-1.03%) ⬇️
models/notification.go 63.88% <0%> (-0.84%) ⬇️
models/error.go 29.23% <0%> (+0.51%) ⬆️
services/pull/pull.go 35.88% <0%> (+1.17%) ⬆️
modules/git/command.go 89.56% <0%> (+2.6%) ⬆️
services/pull/check.go 53.04% <0%> (+4.87%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33c5e5e...8e93fb1. Read the comment docs.

@lafriks lafriks merged commit b8551f8 into go-gitea:master Mar 8, 2020
@cipherboy
Copy link
Contributor Author

@silverwind Yes, I can file the issue when I get back. I was trying to figure out how to add the class earlier but I didn't get far enough.

@cipherboy
Copy link
Contributor Author

@silverwind -- filed as yuin/goldmark#113.

zeripath added a commit that referenced this pull request Mar 22, 2020
Continuing on from #10668 this PR makes goldmark apply the task-list styling to task-lists.
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing space after checkbox
6 participants