Skip to content

docs(fundamentals): display icons in 'frequent app functions' page#1484

Open
spliffone wants to merge 1 commit intomainfrom
docs/show-icon-in-uxwriting
Open

docs(fundamentals): display icons in 'frequent app functions' page#1484
spliffone wants to merge 1 commit intomainfrom
docs/show-icon-in-uxwriting

Conversation

@spliffone
Copy link
Member

@spliffone spliffone commented Feb 10, 2026

fixes #1444

Describe in detail what your merge request does and why. Add relevant
screenshots and reference related issues via Closes #XY or Related to #XY.


@spliffone spliffone requested review from a team and Killusions as code owners February 10, 2026 09:20
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds the necessary configuration to display icons in the documentation, which involves including the Siemens Element icon CSS and adding some helper classes. The changes are generally good, but I've identified an opportunity to improve the robustness of how the icon CSS file path is resolved in plugin.py. My feedback includes a specific suggestion to make the path resolution more dynamic and less dependent on the execution environment's current working directory.


def on_files(self, files: Files, config: Config):
files.append(File('docs-builder.css', dirname(__file__), config.get('site_dir'), config.get('use_directory_urls')))
files.append(File('siemens-element-icons.min.css', './node_modules/@siemens/element-icons/dist/style/', config.get('site_dir'), config.get('use_directory_urls')))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The hardcoded relative path './node_modules/...' is brittle because it relies on the current working directory. While mkdocs typically changes the CWD to the project's root directory, this can be unreliable in different execution environments or with different mkdocs invocation methods.

For better robustness and consistency with other parts of this project (e.g., index.mjs), I recommend dynamically locating the node_modules directory by traversing up the file system. This makes the plugin more resilient.

You could replace the current line with logic like this:

# Find node_modules directory by traversing up from the current working directory
path = os.getcwd()
nm_path = None
for _ in range(10): # Limit search depth to 10 levels
    if os.path.isdir(os.path.join(path, 'node_modules')):
        nm_path = os.path.join(path, 'node_modules')
        break
    parent = os.path.dirname(path)
    if parent == path: # Reached root
        break
    path = parent

if not nm_path:
    raise FileNotFoundError("Could not find 'node_modules' directory.")

icon_css_src_dir = os.path.join(nm_path, '@siemens/element-icons/dist/style/')
files.append(File('siemens-element-icons.min.css', icon_css_src_dir, config.get('site_dir'), config.get('use_directory_urls')))

@github-actions
Copy link

@spliffone spliffone marked this pull request as draft February 10, 2026 10:06
@spliffone spliffone force-pushed the docs/show-icon-in-uxwriting branch from 175fa6a to 8045af1 Compare February 10, 2026 10:45
@spliffone spliffone marked this pull request as ready for review February 10, 2026 11:06
@spliffone spliffone added this to the 49.x milestone Feb 10, 2026
@spliffone spliffone force-pushed the docs/show-icon-in-uxwriting branch from 8045af1 to 91dbfde Compare February 10, 2026 19:26
@github-actions
Copy link

Code Coverage

Copy link
Member

@spike-rabbit spike-rabbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of loading it for every file, please just load it into the affected one.

We already have the style symlinked into the docs.
So just adding <link rel="stylesheet" href="_src/siemens-element-icons.min.css" /> should be enough.

When we finally have the live-preview as a web-component, we can maybe start adding this globally.

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

Successfully merging this pull request may close these issues.

Icons are not displayed in UX writing styleguide

2 participants