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

Panel close button can be hidden behind the panel title #2083

Open
TomHall2020 opened this issue Feb 20, 2025 · 2 comments
Open

Panel close button can be hidden behind the panel title #2083

TomHall2020 opened this issue Feb 20, 2025 · 2 comments

Comments

@TomHall2020
Copy link

When using DjDT on a local project I found that the close button was not working. After digging into it I found the stylesheet I was using for the project (missing.css) was interfering with the positioning on the panel title such that it now fully overlaps the close button and I couldn't click through to the button beneath the heading.

Fixing was as simple as swapping two lines in the template so that the button is rendered after the title h3 element. Is this something that can be included upstream? I didn't look into anything else like messing with z-index, as this just seemed the more natural way to write it.

 {% if panel.has_content and panel.enabled %}
   <div id="{{ panel.panel_id }}" class="djdt-panelContent djdt-hidden">
     <div class="djDebugPanelTitle">
-      <button type="button" class="djDebugClose">×</button>
       <h3>{{ panel.title }}</h3>
+      <button type="button" class="djDebugClose">×</button>
     </div>
     <div class="djDebugPanelContent">
       {% if toolbar.should_render_panels %}
@matthiask
Copy link
Member

This change looks obviously correct to me. I suspect creating a testcase with selenium wouldn't be that straightforward? We generally want changes to include tests, but including fixes is still better than not including them.

I'm slightly unclear if we should also ship more CSS overrides anyway, but #2007 would possibly be a more sustainable way to address the issue.

Long story short: please submit a PR!

@TomHall2020
Copy link
Author

I see the projects already configured to run some tests on selenium. I guess the main issue is that it will pass as things stand without the change, it needs the conflicting css in there to trigger the issue. If I can isolate it to a simple rule that causes the issue I can include that into a variant of the test page template, but as a regression test it would be quite flaky. In that someone could have a different css incompatibility and this test wouldn't necessarily catch it.

I'll do the PR at any rate though, and if its straightforward to include the test I'll do so, but I wont worry too much about it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants