Taxonomy Term List page is broken when users don't have permission to edit/create terms

Created on 13 January 2025, 7 months ago

Problem/Motivation

When user has access to view the list of terms/taxonomy overview page and they don't have access to manage them, the style is broken and confusing.
The user sees a + SVG but no indentation. It's quite hard to distinguish between parent and child terms.

Steps to reproduce

1. Create a taxonomy
2. Add few terms
3. Add children terms so you can get the indentation

Proposed resolution

Adjust the css/styling of the page

πŸ› Bug report
Status

Active

Version

10.4 ✨

Component

Claro theme

Created by

πŸ‡§πŸ‡·Brazil carolpettirossi Campinas - SP

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

Merge Requests

Comments & Activities

  • Issue created by @carolpettirossi
  • πŸ‡³πŸ‡ΏNew Zealand quietone

    Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies β†’ .

  • did u logged in using admin account or its just a normal account without admin access because i am not able to reproduce the issue here.

    please share a short video of the issue. thanks

  • πŸ‡§πŸ‡·Brazil carolpettirossi Campinas - SP

    @saurav-drupal-dev, you must remove the permission to create/edit terms and add the view overview,

    See the permissions here for content editor:

    Evidence that the layout is broken for the user with content editor role:

  • Issue is able replicate on local, Here this issue coming due to styled components as they would be only view only access to editors. Hence does not containing draggable table that doesn't allow svg and indentation styles get rendered, so for Proposed solution is to create a separate component style to overwrite as below.

    /* *
     * @file
     * Styles for custom indentation and tree item visibility for view access.
     */
    .indentation {
        position: relative;
        inset-inline-start: calc(var(--space-xs)* -0.5);
        float: left; /* LTR */
        width: 1.5625rem;
        height: 1.5625rem;
        background: none !important;
        line-height: 0;
    }
    
    [dir="rtl"] .indentation {
        float: right;
    }
    
    .tree__item {
        display: none;
    }

    Did a try on my system it worked for me. (as image attached). not sure so, I would like to explore any other potential solutions to address this. if all good Will Raise MR.

  • @kushagra.goyal What if all the code related to .indention and .tree classes was moved to the new indentation.pcss.css file?
    It looks like this block of code can be completely extracted from the tabledrag.css file.

    /* *
     * @file
     * Styles for custom indentation and tree item visibility for view access.
     */
    
    /**
     * Indentation.
     */
    .indentation {
      position: relative;
      inset-inline-start: calc(var(--space-xs) * -0.5);
      float: left; /* LTR */
      width: 25px;
      height: 25px;
      background: none !important;
      line-height: 0;
    
      @nest .tabledrag-cell-content & {
        /* Fixes Safari bug (16.1 at least) where table rows are overly large when
          using indentation (e.g. re-ordering menu items. */
        display: inline-flex;
        float: none;
        overflow: hidden;
        flex-direction: column;
        height: 100%;
      }
    
      @nest [dir="rtl"] & {
        float: right;
      }
    }
    
    /**
     * Tree is the visual representation for the simultaneously moved draggable
     * rows.
     *
     * These rules are styling the inline SVG that is placed inside the .indentation
     * element.
     */
    .tree {
      width: 25px;
      height: 25px;
    }
    
    .tree__item {
      display: none;
    }
    
    /* LTR tree child. */
    .tree-child path {
      &:not(.tree__item-child-ltr) {
        display: none;
      }
    
      &.tree__item-child-ltr {
        display: block;
      }
    }
    
    /* RTL tree child. */
    [dir="rtl"] {
      & .tree-child path {
        &:not(.tree__item-child-rtl) {
          display: none;
        }
        &.tree__item-child-rtl {
          display: block;
        }
      }
    
      /* Last RTL tree child. */
      & .tree-child-last path {
        &:not(.tree__item-child-last-rtl) {
          display: none;
        }
        &.tree__item-child-last-rtl {
          display: block;
        }
      }
    }
    
    /* Last LTR tree child. */
    .tree-child-last path {
      &:not(.tree__item-child-last-ltr) {
        display: none;
      }
      &.tree__item-child-last-ltr {
        display: block;
      }
    }
    
    /* Horizontal line. */
    .tree-child-horizontal path {
      &:not(.tree__item-horizontal) {
        display: none;
      }
    
      &.tree__item-horizontal {
        display: block;
      }
    }
    

    I have tested your suggestion and I can confirm it's working fine.

  • Hi @brunoalmeida, thanks for looking into this. Yeah It will ok by adding code for tree and indentation As Tree is the visual representation for draggable rows. This will help item visibility for view access.

  • Pipeline finished with Failed
    7 months ago
    Total: 636s
    #415024
  • I needed to fix this in my current project so I have created a MR using the @kushagra.goyal solution.
    I'm also adding a patch for Core version 10.4.x.

  • Pipeline finished with Success
    6 months ago
    Total: 331s
    #434858
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Tried replicating this following the steps but not getting the results mentioned. Think more information is needed. Adding a new file and editing a library probably will need sub-maintainer approval too.

  • Status changed to Needs work 24 days ago
  • First commit to issue fork.
  • Pipeline finished with Failed
    24 days ago
    #562458
  • πŸ‡§πŸ‡ͺBelgium lisotton Brussels

    I resolved the conflict and changed the class "tabledrag-cell-content" in the "indentation.css" to "tabledrag-cell", as the "tabledrag-cell-content" is created by JS when tabledrag is enabled.

    I had the same problem in the menu form when the tabledrag is disabled using a form alter.

    @smustgrave you can replicate the issue creating a menu with some items in different levels and then with a form alter doing:
    unset($form['links']['links']['#tabledrag']);

  • πŸ‡ΊπŸ‡ΈUnited States cboyden

    I was looking at πŸ“Œ Conditionally show Operations, Weight column in Vocabulary, Terms overview Active and posted a comment there, but it looks like this is a better issue for it. The problem isn't only the display, it's also the invalid markup.

    I believe the source of the problem is that the tabledrag SVGs (indentation and handles) and the Weight header are added in code, but their styling and functionality depend on the tabledrag JS and CSS. These assets are not added to the term overview page when the current user doesn't have edit terms permissions, resulting in a broken display and invalid markup.

    Steps to reproduce:

    1. Install the latest Drupal 11.x branch with the standard profile.
    2. Log in as an administrator.
    3. Change the Content Editor role permissions:
      • Remove Create terms and Edit terms in the Tags vocab.
      • Add Access taxonomy overview.
    4. Save permissions.
    5. Create 5 to 10 terms in the Tags vocab and arrange them with three or more levels of depth.
    6. View the Tags overview page.
    7. Note the appearance of the drag handles, the Status column, and the Operations column.
    8. Note that depth is shown by indenting. (BTW this is an accessibility issue.)
    9. Press the Show row weights button.
    10. Note the Weight column is shown and the drag handles are hidden. Terms are still indented to show depth.
    11. Create a user with the Content Editor role.
    12. Log in as the Content Editor.
    13. View the Tags overview page.
    14. Note there is no "Show row weights" button, and both the Operations and Weight columns are empty.
    15. Note the following display issues:
      • Drag handle images are displayed, but they are unstyled and don't do anything.
      • Items deeper than second level show multiple drag handles.
      • Row hover highlight doesn't extend to Weight column.
      • There is no indenting for depth.
    16. Note the following invalid markup:
      • The header row has 4 cells, but each data row only has 3 cells.

    I tested the latest MR and it still produces invalid markup: The header row has 4 cells, but each data row only has 3 cells.

  • Pipeline finished with Failed
    6 days ago
    #576756
  • Pipeline finished with Failed
    6 days ago
    #576771
  • Pipeline finished with Failed
    5 days ago
    Total: 609s
    #577591
  • @cboyden currently, the "Weight" thead is always created. There's only a condition for the label, as you can see below:

    'weight' => !$operations_access ? $this->t('Weight') : NULL,

    The weight column that is created for each term has a different validation:

    if ($update_tree_access->isAllowed()) {
      $form['terms'][$key]['weight'] = [
        '#type' => 'weight',
        '#delta' => $delta,
        '#title' => $this->t('Weight for added term'),
        '#title_display' => 'invisible',
        '#default_value' => $term->getWeight(),
        '#attributes' => ['class' => ['term-weight']],
      ];
    }
    

    My proposal is to create the Weight thead separately, combining the two validations:

    $weight_reset_allowed = !$operations_access && $update_tree_access->isAllowed();
    
    if ($weight_reset_allowed) {
      $form['terms']['#header']['weight'] = $this->t('Weight');
    }
    

    This validation can be used to add any element that has to do with weights and drag and drop.
    I updated the MR with these changes.

  • Pipeline finished with Failed
    5 days ago
    Total: 560s
    #577602
  • πŸ‡ΊπŸ‡ΈUnited States cboyden

    Thanks @brunoalmeida, I tried this out locally and it's working as expected.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Seems like an issue that probably should have test coverage, since the fix was in the taxonomy module wonder if the component should be set to that.

    Also current solution doesn't match the summary where this was originally about adjusting styling

  • πŸ‡ΊπŸ‡ΈUnited States cboyden

    I've updated the issue summary with more info about the invalid markup and a new proposed resolution.

    It is possible that this problem occurs in other places that use tabledrag, but that would require more investigation.

Production build 0.71.5 2024