-
-
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
Fix: Hide divider when labels are hidden #27747
base: main
Are you sure you want to change the base?
Conversation
This is ready for review |
@@ -206,6 +206,8 @@ function initArchivedLabelFilter() { | |||
for (const label of archivedLabels) { | |||
const id = label.getAttribute('data-label-id'); | |||
toggleElem(label, archivedLabelEl.checked || selectedLabels.includes(id)); | |||
// also toggle the divider | |||
toggleElem($(label).next('.exclusive-scope'), archivedLabelEl.checked || selectedLabels.includes(id)); |
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.
- There is not necessarily a divider
- I think the condition is wrong. It works in the situation you've tested, but I'm fairly certain that in more complex situations it will hide the divider even though it shouldn't.
@@ -691,5 +691,7 @@ export function initArchivedLabelHandler() { | |||
if (!document.querySelector('.archived-label-hint')) return; | |||
for (const label of document.querySelectorAll('[data-is-archived]')) { | |||
toggleElem(label, label.classList.contains('checked')); | |||
// also toggle the divider | |||
toggleElem($(label).next('.exclusive-scope'), label.classList.contains('checked')); |
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 above
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.
Will test it rigorously.
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.
The usecase I imagine to be faulty is two (or more) labels in scope, one archived, the other not
.
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.
The same I was thinking, with org labels as well.
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.
4805034b-ceec-46cd-8631-fca2b25a2d38.mp4
This is the current condition with org and repo labels with all possible usecase,
One is failing, when we have archived label with scoped and normal label with archived marked.
as title
regression #27466
After changes: