Claro: Autocomplete dropdown bleeds through sticky table header

MR has failures

This a problem on 11.x?

I have updated the MR for 11.x, also targeted file was wrong on #8. I have fixed it.

Before/after screenshots should be added to the issue summary so leaving that tag.

Added before/after patch screenshots in issue summary.

It looks like this issue is almost ready to be reviewed and tested by the community. Let's review the issue to ensure that issue summary clearly states the merge request to be committed to 11.x. I added the Novice and Portland2024 tags.

Novice issue reserved for the Mentored Contribution during DrupalCon Portland 2024 contribution day. After 2024.05.08, this issue returns to being open to all. Thanks

Please also hide the non-canonical MR and check to ensure that the canonical one is mergeable with pipelines passing. Thanks!

I am working on reviewing and testing this issue in the mentoring session at DrupalCon Portland. I will update the issue summary as well.

Non-canonical MR hidden and issue summary updated and corrected. Issue ready to move to rtbc status.

@capysara was mentoring @kd_ace on this issue at DrupalCon Portland 2024. And I have been answering some questions as well this afternoon.

Reviewed #27 and tested with @kd_ace, issue looks good for rtbc status.

Okay, I went over this again.

1, @jvest, could you rewrite your comment to clarify the steps you took to verify @kd_ace's work?

2, @ultimike pointed out that the merge request is about 29 commits behind. It still passes, but we should probably update the issue fork.

Also, when switching from merge requests we can hide the patches to make the issue summary a little easier to parse for final review. I've done that.

Adding clarification on testing/review:

Used a shared DrupalPod instance
Enabled Authored By
Added several users added so that the list will be long enough when expanded

The overlay issue appearing behind the table heading is fixed.
Also manually changed the z-index back to original to show the original problem of the drop down appearing behind the table heading.

Updated MR again and tests completed successfully. Moving to RTBC!

Left a comment in the MR as to why we need to find something different from the current solution.

The issue is due to this style that is added to sticky table headers:

.position-sticky thead {
    position: sticky;
    z-index: 500;
    top: var(--drupal-displace-offset-top, 0);
}

This high Z-index is only necessary when the header is actually "stuck", but the Z-index is applied in either state. Unfortunately there is no way to target "stuck" elements with CSS only, so this leaves us with two approaches:

1. Use Javascript to determine the coordinates of the table header and conditionally provide a z-index boosting class only when stuck at the top (and accounting for offsets such as toolbar)
2. Update the Z-index of the dropdown, but target something specific to autocomplete vs. the current approach of targeting ui-frontIf an autocomplete dropdown is open, it's reasonably safe to assume it should be above other elements. An autocomplete-specific z-index boost (vs ui-front) is easier to test and less at risk of causing unwanted side effects

Thanks for the comment bnjmnm. I can definitely see where you are coming from and would be happy to help find a solution. I plan on looking into option 2 a bit more and seeing if I can propose a better css change than the current MR.

Thought of a different approach without changing the global css. Decreased the z-index value of the table header based on the content page so that the autocomplete dropdown doesn't hide behind it. Also checked the table header remains sticky on the content pages.
Please review.

MR has a bunch of test failures.

Also if a new approach is going to be used then issue summary will have to be updated and fresh set of screenshots added. Moving to NW for that.

Pushed a rebase for the PR and now the test failures are passed.

The z-indexes in Drupal are documented and considered stable. Custom Themes and modules are built knowing what these core z-indexes are, so reducing the z-index of a sticky header by 400 could be disruptive, where suddenly many elements can bleed through sticky header that were previously covered by it.

Changing the issue title to describe the problem and not a potential solution.

Added z-index according to

Update the Z-index of the dropdown, but target something specific to autocomplete vs. the current approach of targeting ui-frontIf an autocomplete dropdown is open, it's reasonably safe to assume it should be above other elements. An autocomplete-specific z-index boost (vs ui-front) is easier to test and less at risk of causing unwanted side effects

.

Believe the changes addresses the concern by @bnjmnm and still addresses the issue.

LGTM

Thank you to everyone who worked on this issue and got it to RTBC!

I have read the issue summary, comments and the MR. Thank you to @bnjmnm for creating a meaningful title. The title is used in the commit message so this is helpful to anyone going through the git logs. I did not find any unanswered questions. I don't do front end so I can not comment on the validity of the change.

Leaving at RTBC.

Let's use 501 as the zindex instead of 600. 600 is not used anywhere at the moment

Hi
Verified MR !8108 on Drupal Version 11.0.x dev and the patch is getting failed.

Testing steps that are followed:
1. Install Claro theme with Drupal 11.0.x
2. Go to /admin/content page.
3. Searched username in the 'Authored by' filter.
4. Observed the auto-complete behavior.

Testing result:
Was able to replicated the issue but when I was applying the patch it was failing. Adding SS below for reference.

Status : Fail

Thanks

thanks @Gauravvvv There a is a duplicated rule we don't need, after that it's good to go.

Addressed feedbacks.

Updated the issue summary to match the current resolution.

Believe this to be addressed.

Committed and pushed e2d8242830 to 11.x and cabbb9391f to 11.0.x and dac6366fd6 to 10.4.x. Thanks!

Is there a patch for this for Drupal versions below 10.4? patch 7 has a z-index of 500 not 501 there is no patch 8

Here is the patch for 10.3.x, exactly the same as the fix committed in 10.4.x.

Hi,

Thank you for providing the patch for 10.3.x.

Could you please confirm if there are plans to merge this patch into 10.3.x as well? Resolving this issue in the 10.3.x branch would be highly beneficial for projects currently relying on that version.

Looking forward to your response!

Best regards.