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

Remove fomantic checkbox module #30162

Merged
merged 19 commits into from Mar 29, 2024
Merged

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 28, 2024

CSS is pretty slim already and the .ui.toggle.checkbox sliders on admin page also still work. The only necessary JS is the one that links input and label so that it can be toggled via label. All checkboxes except the markdown ones render at --checkbox-size: 16px now.

Screenshot 2024-03-28 at 22 15 10 Screenshot 2024-03-28 at 21 00 07 Screenshot 2024-03-28 at 22 14 34

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 28, 2024
@pull-request-size pull-request-size bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 28, 2024
@github-actions github-actions bot added modifies/frontend modifies/templates This PR modifies the template files modifies/js labels Mar 28, 2024
@silverwind silverwind added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 28, 2024
@silverwind
Copy link
Member Author

WIP because I noticed this JS needs to be removed/updated:

web_src/js/features/repo-issue.js:291:  $checkbox.checkbox({
web_src/js/features/repo-issue.js:293:      const checked = $checkbox.checkbox('is checked');
web_src/js/features/repo-issue.js:296:      $checkbox.checkbox('set disabled');
web_src/js/features/repo-issue.js:308:        $checkbox.checkbox('set enabled');
web_src/js/features/admin/common.js:225:          $checkboxes.checkbox('check');
web_src/js/features/admin/common.js:228:          $checkboxes.checkbox('uncheck');
web_src/js/features/admin/common.js:231:          $checkboxes.checkbox('toggle');
web_src/js/features/admin/common.js:240:        if ($(this).checkbox('is checked')) {

Also, web_src/js/modules/fomantic/aria.md needs updates.

@silverwind silverwind marked this pull request as draft March 28, 2024 20:05
@silverwind silverwind marked this pull request as ready for review March 28, 2024 21:06
@silverwind
Copy link
Member Author

Ready now.

@silverwind silverwind added the backport/v1.22 This PR should be backported to Gitea 1.22 label Mar 28, 2024
Comment on lines -241 to +248
data.append('ids[]', this.getAttribute('data-id'));
for (const checkbox of checkboxes) {
if (checkbox.checked) {
data.append('ids[]', checkbox.closest('.ui.checkbox').getAttribute('data-id'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did the selector change here from this to a parent element?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selector is for .checkbox input because in the case above it's easier to work directly with input, but in this case the data-attribute is set on .checkbox. So I can optimize one case or the other.

web_src/js/features/repo-issue.js Outdated Show resolved Hide resolved
web_src/js/modules/fomantic/checkbox.js Outdated Show resolved Hide resolved
silverwind and others added 2 commits March 29, 2024 00:14
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: delvh <dev.lh@web.de>
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 29, 2024
@silverwind silverwind merged commit 8fd1599 into go-gitea:main Mar 29, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 29, 2024
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 29, 2024
CSS is pretty slim already and the `.ui.toggle.checkbox` sliders on
admin page also still work. The only necessary JS is the one that links
`input` and `label` so that it can be toggled via label. All checkboxes
except the markdown ones render at `--checkbox-size: 16px` now.

<img width="174" alt="Screenshot 2024-03-28 at 22 15 10"
src="https://github.com/go-gitea/gitea/assets/115237/3455c1bb-166b-47e4-9847-2d20dd1f04db">

<img width="499" alt="Screenshot 2024-03-28 at 21 00 07"
src="https://github.com/go-gitea/gitea/assets/115237/412be2b3-d5a0-478a-b17b-43e6bc12e8ce">

<img width="83" alt="Screenshot 2024-03-28 at 22 14 34"
src="https://github.com/go-gitea/gitea/assets/115237/d8c89838-a420-4723-8c49-89405bb39474">

---------

Co-authored-by: delvh <dev.lh@web.de>
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Mar 29, 2024
@lunny lunny removed the backport/v1.22 This PR should be backported to Gitea 1.22 label Mar 29, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 29, 2024
* upstream/main:
  Remove fomantic checkbox module (go-gitea#30162)
  Refactor topic Find functions and add more tests for pagination (go-gitea#30127)
  replace jquery-minicolors with coloris (go-gitea#30055)
  Add API for `Variables` (go-gitea#29520)
  Fix `DEFAULT_SHOW_FULL_NAME=false` has no effect in commit list and commit graph page (go-gitea#30096)
  Fix migration v292 (go-gitea#30153)
  Adjust VS Code debug filename match in .gitignore (go-gitea#30158)
  Prevent re-review and dismiss review actions on closed and merged PRs (go-gitea#30065)
  Render code tags in commit messages (go-gitea#30146)
  Bump `@github/relative-time-element` to v4.4.0 (go-gitea#30154)
  Migrate font-family to tailwind (go-gitea#30118)
  Move from `max( id )` to `max( index )` for latest commit statuses (go-gitea#30076)
  Remember login for a month by default (go-gitea#30150)
silverwind added a commit that referenced this pull request Mar 29, 2024
Backport #30162 by @silverwind

CSS is pretty slim already and the `.ui.toggle.checkbox` sliders on
admin page also still work. The only necessary JS is the one that links
`input` and `label` so that it can be toggled via label. All checkboxes
except the markdown ones render at `--checkbox-size: 16px` now.

<img width="174" alt="Screenshot 2024-03-28 at 22 15 10"
src="https://github.com/go-gitea/gitea/assets/115237/3455c1bb-166b-47e4-9847-2d20dd1f04db">

<img width="499" alt="Screenshot 2024-03-28 at 21 00 07"
src="https://github.com/go-gitea/gitea/assets/115237/412be2b3-d5a0-478a-b17b-43e6bc12e8ce">

<img width="83" alt="Screenshot 2024-03-28 at 22 14 34"
src="https://github.com/go-gitea/gitea/assets/115237/d8c89838-a420-4723-8c49-89405bb39474">

Co-authored-by: silverwind <me@silverwind.io>
Co-authored-by: delvh <dev.lh@web.de>
lafriks pushed a commit that referenced this pull request Mar 31, 2024
Fix #30185, regression from
#30162.

The checkboxes were unclickable because the label was positioned over
the checkbox with `padding`. Now it uses `margin` so the checkbox itself
will be clickable in all cases.

Secondly, I changed the for/id linking to also add missing `for`
attributes when `id` is present. The other way around (only `for`
present) is currently not handled and I think there are likey no
occurences in the code and introducing new non-generated `id`s might
cause problems elsewhere if we do, so I skipped on that.
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 31, 2024
Fix go-gitea#30185, regression from
go-gitea#30162.

The checkboxes were unclickable because the label was positioned over
the checkbox with `padding`. Now it uses `margin` so the checkbox itself
will be clickable in all cases.

Secondly, I changed the for/id linking to also add missing `for`
attributes when `id` is present. The other way around (only `for`
present) is currently not handled and I think there are likey no
occurences in the code and introducing new non-generated `id`s might
cause problems elsewhere if we do, so I skipped on that.
silverwind added a commit that referenced this pull request Mar 31, 2024
Backport #30195 by @silverwind

Fix #30185, regression from
#30162.

The checkboxes were unclickable because the label was positioned over
the checkbox with `padding`. Now it uses `margin` so the checkbox itself
will be clickable in all cases.

Secondly, I changed the for/id linking to also add missing `for`
attributes when `id` is present. The other way around (only `for`
present) is currently not handled and I think there are likey no
occurences in the code and introducing new non-generated `id`s might
cause problems elsewhere if we do, so I skipped on that.

Co-authored-by: silverwind <me@silverwind.io>
silverwind added a commit that referenced this pull request Apr 7, 2024
Fix the checkbox issues in
#30303 which were existing
problems with these selectors, but made visible with
#30162.

There is a lot of dead/useless CSS in `form.css`, I only fixed the two
problems and remove CSS that was definitely not in use or needed.

<img width="369" alt="Screenshot 2024-04-06 at 18 00 08"
src="https://github.com/go-gitea/gitea/assets/115237/720f178b-1b22-48d4-8704-becb8ce66129">
<img width="405" alt="Screenshot 2024-04-06 at 18 00 28"
src="https://github.com/go-gitea/gitea/assets/115237/61c0f8ec-34af-46c5-a3fa-7c5c4d30c7d2">

Co-authored-by: Giteabot <teabot@gitea.io>
GiteaBot added a commit to GiteaBot/gitea that referenced this pull request Apr 7, 2024
Fix the checkbox issues in
go-gitea#30303 which were existing
problems with these selectors, but made visible with
go-gitea#30162.

There is a lot of dead/useless CSS in `form.css`, I only fixed the two
problems and remove CSS that was definitely not in use or needed.

<img width="369" alt="Screenshot 2024-04-06 at 18 00 08"
src="https://github.com/go-gitea/gitea/assets/115237/720f178b-1b22-48d4-8704-becb8ce66129">
<img width="405" alt="Screenshot 2024-04-06 at 18 00 28"
src="https://github.com/go-gitea/gitea/assets/115237/61c0f8ec-34af-46c5-a3fa-7c5c4d30c7d2">

Co-authored-by: Giteabot <teabot@gitea.io>
silverwind added a commit that referenced this pull request Apr 7, 2024
Backport #30308 by @silverwind

Fix the checkbox issues in
#30303 which were existing
problems with these selectors, but made visible with
#30162.

There is a lot of dead/useless CSS in `form.css`, I only fixed the two
problems and remove CSS that was definitely not in use or needed.

<img width="369" alt="Screenshot 2024-04-06 at 18 00 08"
src="https://github.com/go-gitea/gitea/assets/115237/720f178b-1b22-48d4-8704-becb8ce66129">
<img width="405" alt="Screenshot 2024-04-06 at 18 00 28"
src="https://github.com/go-gitea/gitea/assets/115237/61c0f8ec-34af-46c5-a3fa-7c5c4d30c7d2">

Co-authored-by: silverwind <me@silverwind.io>
AvengerMoJo pushed a commit to AvengerMoJo/gitea that referenced this pull request Apr 8, 2024
Fix the checkbox issues in
go-gitea#30303 which were existing
problems with these selectors, but made visible with
go-gitea#30162.

There is a lot of dead/useless CSS in `form.css`, I only fixed the two
problems and remove CSS that was definitely not in use or needed.

<img width="369" alt="Screenshot 2024-04-06 at 18 00 08"
src="https://github.com/go-gitea/gitea/assets/115237/720f178b-1b22-48d4-8704-becb8ce66129">
<img width="405" alt="Screenshot 2024-04-06 at 18 00 28"
src="https://github.com/go-gitea/gitea/assets/115237/61c0f8ec-34af-46c5-a3fa-7c5c4d30c7d2">

Co-authored-by: Giteabot <teabot@gitea.io>
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Apr 9, 2024
Fix go-gitea/gitea#30185, regression from
go-gitea/gitea#30162.

The checkboxes were unclickable because the label was positioned over
the checkbox with `padding`. Now it uses `margin` so the checkbox itself
will be clickable in all cases.

Secondly, I changed the for/id linking to also add missing `for`
attributes when `id` is present. The other way around (only `for`
present) is currently not handled and I think there are likey no
occurences in the code and introducing new non-generated `id`s might
cause problems elsewhere if we do, so I skipped on that.

(cherry picked from commit 640850e15f56bbe01f5d8ea407f99c79dc38457e)
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Apr 10, 2024
Backport #30195 by @silverwind

Fix go-gitea/gitea#30185, regression from
go-gitea/gitea#30162.

The checkboxes were unclickable because the label was positioned over
the checkbox with `padding`. Now it uses `margin` so the checkbox itself
will be clickable in all cases.

Secondly, I changed the for/id linking to also add missing `for`
attributes when `id` is present. The other way around (only `for`
present) is currently not handled and I think there are likey no
occurences in the code and introducing new non-generated `id`s might
cause problems elsewhere if we do, so I skipped on that.

Co-authored-by: silverwind <me@silverwind.io>
(cherry picked from commit 9d38c4d60ef5bd015e1430386e38d9f32e050f8f)
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Apr 16, 2024
Fix the checkbox issues in
go-gitea/gitea#30303 which were existing
problems with these selectors, but made visible with
go-gitea/gitea#30162.

There is a lot of dead/useless CSS in `form.css`, I only fixed the two
problems and remove CSS that was definitely not in use or needed.

<img width="369" alt="Screenshot 2024-04-06 at 18 00 08"
src="https://github.com/go-gitea/gitea/assets/115237/720f178b-1b22-48d4-8704-becb8ce66129">
<img width="405" alt="Screenshot 2024-04-06 at 18 00 28"
src="https://github.com/go-gitea/gitea/assets/115237/61c0f8ec-34af-46c5-a3fa-7c5c4d30c7d2">

Co-authored-by: Giteabot <teabot@gitea.io>
(cherry picked from commit 644ade5ae6805a3db063b3f81a596febe49bacaf)
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this pull request Apr 16, 2024
Backport #30308 by @silverwind

Fix the checkbox issues in
go-gitea/gitea#30303 which were existing
problems with these selectors, but made visible with
go-gitea/gitea#30162.

There is a lot of dead/useless CSS in `form.css`, I only fixed the two
problems and remove CSS that was definitely not in use or needed.

<img width="369" alt="Screenshot 2024-04-06 at 18 00 08"
src="https://github.com/go-gitea/gitea/assets/115237/720f178b-1b22-48d4-8704-becb8ce66129">
<img width="405" alt="Screenshot 2024-04-06 at 18 00 28"
src="https://github.com/go-gitea/gitea/assets/115237/61c0f8ec-34af-46c5-a3fa-7c5c4d30c7d2">

Co-authored-by: silverwind <me@silverwind.io>
(cherry picked from commit d26ec5f2eb74a6437f998ab0af2ae058a64e06a2)
@silverwind silverwind deleted the rmcheckbox branch April 16, 2024 19:31
@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/docs modifies/js modifies/templates This PR modifies the template files size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants