[PP1] Refactor breadcrumb

Created on 8 October 2023, 8 months ago
Updated 26 April 2024, about 2 months ago

Problem/Motivation

The current breadcrumb implementation has several bugs and it's having issues with Claro styles.

Example issue where the "Back to site" gets the wrong styles.

Proposed resolution

  • Change breadcrumb naming
  • Fix issues with current typing

Remaining tasks

User interface changes

πŸ“Œ Task
Status

Closed: outdated

Version

1.0

Component

Code

Created by

πŸ‡ͺπŸ‡ΈSpain ckrina Barcelona

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @ckrina
  • Assigned to claireristow
  • πŸ‡¨πŸ‡¦Canada claireristow

    Working on this now!

    Confirmed in slack, the breadcrumb's "back to site" link is missing the back arrow on the left and the pipe on the right (see screenshot). Currently unsure what "Fix issues with current typing" was about so I'll leave that piece of feedback for now.

  • @claireristow opened merge request.
  • Status changed to Needs review 8 months ago
  • πŸ‡¨πŸ‡¦Canada claireristow

    Ready for review!

    To test, visit a create node page (eg. https://mr121-92dvfb1kuzo3pucqszjvehwaiwtuwlfm.tugboatqa.com/en/node/add...). Ensure the breadcrumb's "back to site" arrow and pipe are showing.

    @ckrina, if you remember what proposed resolution #2 was about, let me know and I can push a fix for that as well.

  • Issue was unassigned.
  • Status changed to Needs work 8 months ago
  • πŸ‡·πŸ‡ΈSerbia finnsky

    Thank you for you work here!

    Few notes:

    1. When I remove Claro breadcrumbs.css i got breadcrumbs broken.
    https://gyazo.com/3913fcf7cb6b644992cdc2fc78e6d4f0
    I think it should be independent from any theme.

    2. Instead of using `icons::before` in absolute position imo better to use `display:flex` with `gap` for parent element.

    3. We still use some variables from other places `var(--color-text); var(--color-link-hover)`.
    Should be scoped with module variables.

    4. Please run `yarn lint:css --fix` before commit.

    Also couple of moments in MR overview. Please take a look.

  • First commit to issue fork.
  • πŸ‡·πŸ‡ΈSerbia finnsky

    Added small code example how we can use :where

    https://jsfiddle.net/finnsky/uhmgrj68/7/

    We libraryRemove claro breadcrumbs styles in module. And in same time will not affect styles of other themes.

    because lower specifity.
    :where(.breadcrumbs .breadcrumbs__link) < breadcrumbs__link

    We may try like this.

  • πŸ‡·πŸ‡ΊRussia kostyashupenko Omsk

    The problem here is that we can't disable all breadcrumbs from all themes. Breadcrumbs can be visible even on user pages. We can try to have simple CSS only for admin toolbar (exclusively) by making classnames unique, so instead of having where(.breadcrumbs .breadcrumbs__link) < breadcrumbs__link we could have only .admin-breadcrumbs {}. Smth like this

  • Status changed to Postponed 7 months ago
  • πŸ‡ͺπŸ‡ΈSpain ckrina Barcelona

    We agreed with @finnsky to postpone the work here until we sort out πŸ“Œ Create a functional prototype of the new top contextual bar Needs work because the work happening there might solve this.

  • Status changed to Closed: outdated about 2 months ago
  • πŸ‡ͺπŸ‡ΈSpain ckrina Barcelona
Production build 0.69.0 2024