Skip to content

Conversation

yardenshoham
Copy link
Member

  • Switched from jQuery attr to plain javascript getAttribute
  • Tested the label edit exclusive checkbox and it works as before

- Switched from jQuery `attr` to plain javascript `getAttribute`
- Tested the label edit exclusive checkbox and it works as before

Signed-off-by: Yarden Shoham <git@yardenshoham.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 24, 2024
@yardenshoham yardenshoham added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Mar 24, 2024
@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 24, 2024
@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 24, 2024
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 24, 2024
@silverwind silverwind enabled auto-merge (squash) March 24, 2024 23:45
@silverwind silverwind merged commit a7d0c5d into go-gitea:main Mar 24, 2024
@GiteaBot GiteaBot added this to the 1.23.0 milestone Mar 24, 2024
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 24, 2024
@yardenshoham yardenshoham deleted the labeledit-jquery-attr branch March 25, 2024 06:28
silverwind added a commit to silverwind/gitea that referenced this pull request Mar 25, 2024
* origin/main:
  Remove fomantic table module (go-gitea#30047)
  Fix menu buttons in issues and release (go-gitea#30056)
  Fix git grep search limit, add test (go-gitea#30071)
  Fix button hover border (go-gitea#30048)
  Fix Add/Remove WIP on pull request title failure (go-gitea#29999)
  Fix misuse of `TxContext` (go-gitea#30061)
  Remove jQuery `.attr` from the reaction selector (go-gitea#30052)
  Remove jQuery `.attr` from the ComboMarkdownEditor (go-gitea#30051)
  Remove jQuery `.attr` from the label edit exclusive checkbox (go-gitea#30053)
  Remove jQuery `.attr` from the repository topic bar (go-gitea#30050)
  Use db.ListOptions directly instead of Paginator interface to make it easier to use and fix performance of /pulls and /issues (go-gitea#29990)
  Migrate `gt-hidden` to `tw-hidden` (go-gitea#30046)
  Forbid jQuery `is` and fix issues (go-gitea#30016)
  Remove fomantic segment module (go-gitea#30042)
  Migrate margin and padding helpers to tailwind (go-gitea#30043)
@lunny lunny modified the milestones: 1.23.0, 1.22.0 Mar 26, 2024
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 23, 2024
if (isExclusiveScopeName(nameInput.value)) {
exclusiveField?.classList.remove('muted');
exclusiveField?.removeAttribute('aria-disabled');
if (exclusiveCheckbox.checked && exclusiveCheckbox.getAttribute('data-exclusive-warn')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Here is a regression bug, the code below uses $exclusiveCheckbox.data('exclusive-warn', ... and can't be read by getAttribute

Copy link
Member

@silverwind silverwind Dec 7, 2024

Choose a reason for hiding this comment

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

.data("foo") is equivalent to .getAttribute("data-foo") for plain string data, no?

Copy link
Contributor

Choose a reason for hiding this comment

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

getAttribute('data-foo') can't read $().data('foo', 'value'), see below $exclusiveCheckbox.data('exclusive-warn',...)

Copy link
Member

Choose a reason for hiding this comment

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

Ah indeed. I forgot that .data() does not actually write HTML attributes. Should have refactored the whole file.

Copy link
Contributor

Choose a reason for hiding this comment

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

-> Refactor LabelEdit #32752

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. modifies/js 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.

6 participants