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

Custom text & anchor support for Mapping Table Column #2531

Open
stefanc18 opened this issue Feb 4, 2025 · 4 comments
Open

Custom text & anchor support for Mapping Table Column #2531

stefanc18 opened this issue Feb 4, 2025 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@stefanc18
Copy link
Contributor

🙋 Feature Request

😯 Problem to Solve

The Systems grid on SLE currently has 2 columns for rendering the job status of a system: an icon column and an anchor text column:

Image

  • The icon column can have multiple states based on the job status; for example, it will be a spinner when the job is in progress, a green checkmark when the job is succeeded or a red xmark if the job failed.
  • The icon column will always have as tooltip the status of the job. So even if there is a very limited set of icons, the text is customized to reflect the exact status of the job as queried from the Systems Management Service.
  • The anchor column is a link to the job details.

The current implementation for the icon column is to use the existing mapping table column. A mapping is done for each system row, so the system id is essentially used as a key. This allows us to have a different tooltip for each system depending on its status. However, this approach has heavy performance issues when the table has 1000 rows and the entire column is populated:

  • With the icon column added to the table:

Image

  • After removing it:

Image

Using the performance tools of Google Chrome we notice that the heavy operation is coming from the table, after the column map is changed:

Image

💁 Proposed Solution

  • Have a textFieldName (or a similar property) that would allow providing a custom text for the column. It would look something like this:

Image

With this approach, the mapping icons would be fixed (the fixed set of possible icons) and only the tooltip text would be given dynamically by a different property of the row.

Moreover, the desire on the systems table is to unite those two columns into one. This would also require another property on the mapping column to allow an URL to be specified, which the Systems table would use to populate with the job details URL for each job status. This would make the text that is displayed next to the column to be clickable and link to the job details page.

📋 Tasks

@stefanc18 stefanc18 added enhancement New feature or request triage New issue that needs to be reviewed labels Feb 4, 2025
@rajsite
Copy link
Member

rajsite commented Feb 4, 2025

In terms of sorting and grouping is this a mapping column with hyperlink text decoration or a hyperlink text column with mapping decoration?

I'm guessing it's more of the former, i.e. the state represented by the icon is the interesting thing to sort and group by and the hyperlink text is metadata.

@stefanc18
Copy link
Contributor Author

stefanc18 commented Feb 5, 2025

I agree, for the systems table use case, I think it makes much more sense to group by the icon state (jobs which are in progress / completed / failed / etc) than by the text mentioning which job / the progress status. The same philosophy would be applied to sorting. I imagine this would also apply to other use cases as well. @TJ-G

@rajsite
Copy link
Member

rajsite commented Feb 5, 2025

Chatted with @atmgrifter00 and @jattasNI about what the API would look like and the proposal is:

  • A new column type, something like nimble-table-column-mapping-anchor
    • The idea being that we are introducing a new way to visualize a set of mappings (as interactable anchors).
      • So nimble-table-column-mapping stays as non-interactive renderings of mappings.
      • The nimble-table-column-mapping-anchor is interactable mappings that expose anchors.
      • Keeps the door open for future interactable types like nimble-table-column-mapping-button or nimble-table-column-mapping-menu-button (☠). We should capture this mapping column convention of the column type representing the interaction (non-interactive, anchor, button) of the different mapping visuals (icon, text, spinner, empty).
    • The api surface for anchor related things should match nimble-table-column-anchor, i.e. href-field-name, label-field-name, underline-hidden, placeholder
    • The api surface of mapping related things should match nimble-table-column-mapping, i.e. field-name, key-type, but not width-mode (?)
  • The column would only support nimble-mapping-icon, nimble-mapping-spinner, and nimble-mapping-empty
  • Visual design questions:
    • How should icon text mappings with a hyperlink be visualized:
      Image
  • Angular navigation guard would be added (following the nimble-table-column-anchor pattern for that directive)

Alternatives:

  • Add hyperlink features to current nimble-table-column-mapping (href-field-name, label-field-name, underline-hidden, placeholder)
    • cons: Does that mean all possible mappings need to have a hyperlink representation? If there are different interaction modes it all gets dumped on the same column api? Adds anchor API features unnecessarily for mappings that don't need the interaction.
  • Create a mapping type specific to this case like nimble-mapping-icon-anchor
    • cons: Similar to above, forces the nimble-table-column-mapping to have an API surface dictated by this option type that might not even be used. Also means that anchor API surface is duplicated on every option which is unlikely to be useful in practice (href-field-name, label-field-name, underline-hidden, placeholder)

Sounds like @atmgrifter00 plans to make an HLD proposal for the new column type next.

@atmgrifter00
Copy link
Contributor

Yeah, my proposal is to not include width-mode as the use-case for this column is to explicitly provide text content in the cell alongside an (optional?) icon.

@m-akinc m-akinc removed the triage New issue that needs to be reviewed label Feb 6, 2025
@m-akinc m-akinc moved this to Current Iteration in Nimble Design System Priorities Feb 6, 2025
@m-akinc m-akinc moved this from Current Iteration to In Progress in Nimble Design System Priorities Feb 6, 2025
jattasNI added a commit that referenced this issue Mar 7, 2025
# Pull Request

## 🤨 Rationale

The impetus for #2531 was performance issues due to creating a mapping
element for each row of table data. We should explicitly suggest not
doing this.

## 👩‍💻 Implementation

Added usage guidance to the table column mapping story.

## 🧪 Testing

Inspect storybook build

## ✅ Checklist

<!--- Review the list and put an x in the boxes that apply or ~~strike
through~~ around items that don't (along with an explanation). -->

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.

---------

Co-authored-by: Milan Raj <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: In Progress
Development

No branches or pull requests

4 participants