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.

Sign up for GitHub

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

Jump to bottom

use ellipsis to truncate breadcrumbs #1583

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

use ellipsis to truncate breadcrumbs #1583

wants to merge 2 commits into from
+15 −4

Conversation

drammock
Copy link
Collaborator

@drammock drammock commented Dec 5, 2023 •
edited

WIP to address #1568. Doesn't work. The below proof-of-concept with the same tag structure / depth of nesting does work, so I'm a bit at a loss as to where the crucial difference is.

If anyone finds it helpful, the below HTML reconstructs the basic node structure of our theme. As written it doesn't work, but seems to work fine as long as you remove display: flex from any one of the outer flex nodes (anything parent to the ul node).

<html>
    <head>
        <style>
            body > div {
                background-color: yellow;
            }
            .FLEX {
                display: flex;
            }
            .UL {
                display: flex;
            }
            .LI {
                display: flex;
            }
            .LI::before {
                content: ">";
            }
            .LI:last-child {
                overflow:hidden;
            }
            .LI > span {
                overflow-x: hidden;
                text-overflow: ellipsis;
                min-width: 0;
                white-space: nowrap;
            }
        </style>

    </head>
    <body>
        <div class="FLEX"> <!-- bd-container -->
            <div class="FLEX">  <!-- bd-container__inner -->
                <main class="FLEX">
                    <div class="FLEX">  <!-- bd-content -->
                        <div class="FLEX">  <!-- bd-article-container -->
                            <div>  <!-- bd-header-article -->
                                <div class="FLEX"> <!-- header-article__inner header-article-items -->
                                    <div class="FLEX"> <!-- header-article-items__start -->
                                        <div>  <!-- header-article-item -->
                                            <nav>
                                                <ul class="UL">
                                                    <li class="LI">0123456789</li>
                                                    <li class="LI">ABCDE</li>
                                                    <li class="LI ACTIVE">
                                                        <span>abcdefg hijklmnop qrstuv wxyz</span>
                                                    </li>
                                                </ul>
                                            </nav>
                                        </div>
                                    </div>
                                </div>
                            </div>
                        </div>
                    </div>
                </main>
            </div>
        </div>
    </body>
</html>

@drammock drammock added the kind: bug Something isn't working label Dec 5, 2023
@drammock
Copy link
Collaborator Author

drammock commented Dec 6, 2023

ok, the feature works now on https://pydata-sphinx-theme--1583.org.readthedocs.build/en/1583/community/setup.html (personally I needed to zoom in a lot or turn on the browser dev tools pane to get the window narrow enough to see the effect; on smaller screens that shouldn't be necessary).

However, I had to remove display: flex from two container nodes in order to get this to work. This likely breaks something under some circumstances, but I'm not familiar enough with flex to predict which cases should be checked --- multiple content templates in the header is probably one case to look at, but there may be others. So this now works but needs review/testing. @ivanov do you have relevant expertise here?

@ivanov
Copy link

ivanov commented Dec 6, 2023

This is fantastic, @drammock - thank you! My trick for checking is to inspect the breadcrumb node in questions and then paste its own content multiple times in there.

image

I'm setting this up locally right now to play with it, specifically I'm going to try to apply the logic to the breadcrumb-item class directly, without having to wrap inside an ellipsis span. But I'm a front-end impostor, at best, so we might have to cha-cha and back and forth by trying this and waiting to hear back about under which situation it breaks.

@drammock
Copy link
Collaborator Author

drammock commented Dec 7, 2023 •
edited

I'm going to try to apply the logic to the breadcrumb-item class directly, without having to wrap inside an ellipsis span.

In my tests this didn't work, but maybe you'll have better luck (I think there was a SO post that explained why it wouldn't work but I'm on mobile and can't easily find it right now)

EDIT: https://css-tricks.com/flexbox-truncated-text/ (though cf https://codepen.io/psyrendust/pen/ZWPxmM/)

@12rambau
Copy link
Collaborator

12rambau commented Jan 3, 2024

It is working as expected on my side could you describe what is preventing you from making this PR ready to merge ?

@drammock
Copy link
Collaborator Author

drammock commented Jan 3, 2024 •
edited

It is working as expected on my side could you describe what is preventing you from making this PR ready to merge ?

I haven't thoroughly tested that things don't look weird under circumstances that don't occur in our own docs / tests. Specifically (as I mentioned above) I'm worried about user sites that put multiple items into article_header_start and/or article_header_end (we use theme default which is only 1 item, the breadcrumbs, in article_header_start, and article_header_end is empty). As I write this, I realize that if someone put the breadcrumbs somewhere else besides the article header, then the ellipsis overflow might not work (putting breadcrumbs elsewhere is also not tested AFAIK).

@drammock drammock mentioned this pull request Jan 5, 2024
Open
Charlie-XIAO added a commit to Charlie-XIAO/scikit-learn that referenced this pull request Jan 6, 2024
@Charlie-XIAO
[Patch24] revise supervised-learning page; vendored template to fix b…
31a474b 
…readcrumbs, see pydata/pydata-sphinx-theme#1583
@Charlie-XIAO Charlie-XIAO mentioned this pull request Jan 16, 2024
Closed
@drammock drammock mentioned this pull request Jan 16, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
kind: bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@drammock @ivanov @12rambau