Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Convert issue list checkboxes to native #23596

Merged
merged 6 commits into from
Mar 30, 2023
Merged

Convert issue list checkboxes to native #23596

merged 6 commits into from
Mar 30, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 20, 2023 •
edited
Loading

Use native instead of fomantic checkboxes in issue list. Benefits include no more JS pop-in on load and perfect a11y.

Before, with JS pop-in:

Screenshot 2023-03-20 at 17 02 02

After, Firefox on macOS:

Screenshot 2023-03-20 at 17 01 26

After, Chrome on macOS:

Screenshot 2023-03-20 at 17 01 42

I opted to not do styling yet but I see that the inconsistency between browsers may already be reason enough on doing it. I think if we style them, there should be one global style, including markdown ones which currently have custom styling.

@silverwind silverwind added type/enhancement An improvement of existing functionality topic/ui Change the appearance of the Gitea UI labels Mar 20, 2023
@silverwind silverwind mentioned this pull request Mar 20, 2023
Merged
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 20, 2023
@silverwind
Copy link
Member Author

silverwind commented Mar 20, 2023 •
edited
Loading

As for styling, I think doing something like Firefox does, but with thinner border on unchecked state would be my preference.

Edit: marking as draft until styling is in.

@silverwind silverwind added this to the 1.20.0 milestone Mar 20, 2023
@silverwind silverwind marked this pull request as draft March 20, 2023 16:28
@silverwind
Copy link
Member Author

silverwind commented Mar 23, 2023 •
edited
Loading

Some WIP on checkbox styling: https://jsfiddle.net/silverwind/g58fr0zm/

@wxiaoguang
Copy link
Contributor

I opted to not do styling yet but I see that the inconsistency between browsers may already be reason enough on doing it. I think if we style them, there should be one global style, including markdown ones which currently have custom styling.

IMO I think the native style is good enough, then we do not need to fix these styles again and again, it saves everyone's time.

@delvh
Copy link
Member

delvh commented Mar 25, 2023

Edit: marking as draft until styling is in.

I agree with @wxiaoguang, I don't think we need to make it overly complicated, I'm already happy to get rid of some Fomantic UI checkboxes

@silverwind silverwind marked this pull request as ready for review March 25, 2023 17:09
@silverwind
Copy link
Member Author

silverwind commented Mar 25, 2023 •
edited
Loading

Well, we can land this as-is. I plan to follow up with checkbox module removal later, and with that, I guess I would also remove the custom checkbox styles on markup.

Still, I do think I prefer styling these checkboxes eventually, the Chrome ones look especially bad imho😉

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Mar 25, 2023
@delvh
Copy link
Member

delvh commented Mar 29, 2023

ping for other maintainers.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 29, 2023 •
edited
Loading

ping for other maintainers.

Hmm, although not for me, but the comment #23596 (comment) is not resolved yet

@silverwind
Copy link
Member Author

silverwind commented Mar 29, 2023 •
edited
Loading

I'll do the title removal. autocomplete attribute will stay.

@silverwind
Copy link
Member Author

Done and rebased. Should be ready now.

@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 30, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 30, 2023
@jolheiser jolheiser enabled auto-merge (squash) March 30, 2023 14:08
@jolheiser jolheiser merged commit 525b738 into go-gitea:main Mar 30, 2023
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 30, 2023
@silverwind silverwind deleted the issue-checkboxes branch March 30, 2023 15:14
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 31, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
c9ca0a0 
* upstream/main:
  [skip ci] Updated translations via Crowdin
  Fix dropdown direction behavior (go-gitea#23806)
  Fix incorrect/Improve error handle in edit user page (go-gitea#23805)
  Fix "Updating branch by merge" bug in "update_branch_by_merge.tmpl" (go-gitea#23790)
  Fix incorrect visibility dropdown list in add/edit user page (go-gitea#23804)
  Convert issue list checkboxes to native (go-gitea#23596)
  Fix checks for `needs` in Actions (go-gitea#23789)
  Diff improvements (go-gitea#23553)
  [Patch] Fix closed PR also triggers Webhooks and actions (go-gitea#23782)
  Improve backport-locales.go (go-gitea#23807)
  [skip ci] Updated translations via Crowdin
  Refactor commit status for Actions jobs (go-gitea#23786)
  Add ONLY_SHOW_RELEVANT_REPOS back, fix explore page bug, make code more strict (go-gitea#23766)
  Don't apply the group filter when listing LDAP group membership if it is empty (go-gitea#23745)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@wxiaoguang wxiaoguang wxiaoguang left review comments

@lunny lunny lunny approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
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 type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
6 participants
@silverwind @wxiaoguang @delvh @lunny @GiteaBot @jolheiser