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

Review the theme for Wazuh dashboard based on OSD 2.18.0 #423

Closed
Desvelao opened this issue Nov 20, 2024 · 4 comments · Fixed by #546
Closed

Review the theme for Wazuh dashboard based on OSD 2.18.0 #423

Desvelao opened this issue Nov 20, 2024 · 4 comments · Fixed by #546
Assignees
Labels
level/task Task issue type/bug Bug issue

Comments

@Desvelao
Copy link
Member

Describe the bug

The v7 theme (used in previous version of Wazuh dashboard) has a visual bug in some button hidding part of the text.

image

The v9 theme seems to be similar to v7 and does not contain the mentioned bug of v7.

OSD 2.18.0 using the v7 theme:
image

OSD 2.18.0 using the v9 theme, the problem is not present:
image

We could consider the usage of v9 theme by default or fix the bug on v7.

@Desvelao
Copy link
Member Author

Desvelao commented Feb 21, 2025

Research

This problem is happening in OpenSearch Dashboards 2.19.0 when using v7 theme and this seems to be related with styles of OUI library v1.19.0.

The problem seems to be located in empty buttons. These have a style line-height: inherit that could be causing the problem where the text is cut-off.

Image

Redefining that property or removing that definition (fallback to line-height: 1.5), causes the text is correctly displayed.

Image

I found a related issue where there is a discussion about the possible fix: opensearch-project/oui#1486. I guess we could add the fix to the generated styles in the served bundles/osd-ui-shared-deps/osd-ui-shared-deps.v7.light.css (and dark mode) because this is a generated file after the build of Wazuh dashboard core. This pull request (opensearch-project/oui#1490) proposes the deletion of line-height: inherit from .euiButtonEmpty and the addition of display: flex; to .euiButtonEmpty__text.

Possible solutions

- 'osd-ui-shared-deps.v7.dark': ['@elastic/eui/dist/eui_theme_dark.css'],
- 'osd-ui-shared-deps.v7.light': ['@elastic/eui/dist/eui_theme_light.css'],
+ 'osd-ui-shared-deps.v7.dark': [Path.resolve('./v7theme-fix/eui_theme_dark.css')],
+ 'osd-ui-shared-deps.v7.light': [Path.resolve('./v7theme-fix/eui_theme_light.css')],

Then rebuild with yarn osd:bootstrap the pacakge or the Wazuh dashboard.

  • Fix the generated style files after the build of the Wazuh dashboard core. Replace packages/osd-ui-shared-deps/osd-ui-shared-deps.v7.light.css and packages/osd-ui-shared-deps/osd-ui-shared-deps.v7.dark.css
  • Change to another theme by default that has no the problem such as:
    • next (v8):
      Image

    Note this has different primary color

    • v9.
      Image
  • Overwriting from the plugins side the style for .euiButtonEmpty__text:
.euiButtonEmpty__text {
  line-height: normal;
}

If the style is not loaded and the user access to a core plugin of Wazuh dashboard, then the user could see the style problem. According to opensearch-project/oui#1486 (comment), the proposed solution line-height: normal could cause regression of the fix that added the line-height: inherit.

Additional context

In the bootrap.js file, this loads the resources files (styles, js files, etc...) and load a set of files related to the selected theme. They are defined here:

themeTagStyleSheetPaths[themeTag] = [
`${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.baseCssDistFilename}`,
`${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.themeCssDistFilenames[effectiveVersion][effectiveMode]}`,
`${basePath}/node_modules/@osd/ui-framework/dist/${UiSharedDeps.kuiCssDistFilenames[effectiveVersion][effectiveMode]}`,
`${basePath}/ui/legacy_${mode}_theme.css`,
];
Adding the fix to some file related to the affected theme version or adding an additional file with the fix could solve the problem. The latest file is shared between the different theme versions, and this could be used to include the fix applygin to all the themes. But, the problem was only seen in the v7 so makes sense only applying the fix to this version.

@Desvelao
Copy link
Member Author

Desvelao commented Feb 24, 2025

Solution based on replacing the v7 theme files adding the fix

I am working into adding the v7 theme files (light and dark) to the packages/osd-ui-shared-deps as local files and replacing the wepack configurtation as commented here #423 (comment).

I tried in a pre-built image of Wazuh dashboard 4.12.0 and applying the changes solved the problem.

I commited the changes to the branch bug/423-v7-theme-empty-button-text-cut-off and built a local Wazuh dashboard dev Docker image following these instructions (a537e76#diff-3490ac4862df698e7363e6cb85f50dcf76935a4ef1714187d614b94ce4e8f1c4) to test.

docker build   --build-arg NODE_VERSION=18.19.0   --build-arg OPENSEARCH_DASHBOARD_VERSION=2.19.0.0   --build-arg WAZUH_DASHBOARD_BRANCH=bug/423-v7-theme-empty-button-text-cut-off   --build-arg WAZUH_DASHBOARD_SECURITY_BRANCH=4.12.0   --build-arg WAZUH_DASHBOARD_REPORTING_BRANCH=4.12.0   --build-arg WAZUH_DASHBOARD_PLUGINS_BRANCH=4.12.0   -t wzd-local:issue-423-patch-v7-theme-empty-button   -f wzd.dockerfile .

Using the Wazuh dashboard dev with the changes, they are applied correctly:

Image
Image

@Desvelao
Copy link
Member Author

In a meeting with @asteriscos , we discussing about adding the styles patch in a new file that should be loaded when selecting the specific theme v7.

This approach avoids maintaining the distributable of v7 (light and dark) theme in the wazuh-dashboard repository.

@Desvelao Desvelao linked a pull request Feb 24, 2025 that will close this issue
7 tasks
@Desvelao
Copy link
Member Author

Desvelao commented Feb 24, 2025

Solution based on adding a new file to load for v7 themes (light and dark modes)

I am working in a different approach based on adding a new style files for v7 theme (light and dark modes) that include the fix.

This file should be added conditionally here:

themeTagStyleSheetPaths[themeTag] = [
`${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.baseCssDistFilename}`,
`${regularBundlePath}/osd-ui-shared-deps/${UiSharedDeps.themeCssDistFilenames[effectiveVersion][effectiveMode]}`,
`${basePath}/node_modules/@osd/ui-framework/dist/${UiSharedDeps.kuiCssDistFilenames[effectiveVersion][effectiveMode]}`,
`${basePath}/ui/legacy_${mode}_theme.css`,
];
pointing to the exposed path of the file. I will use the ${basePath}/ui/{file} defining the file at

Tested using the Docker dev environment:

Image

Related pull request: #546

Tested building a local Wazuh dashboard deb package:

Image

@wazuhci wazuhci moved this from In progress to Pending review in XDR+SIEM/Release 4.12.0 Feb 25, 2025
@wazuhci wazuhci moved this from Pending review to Pending final review in XDR+SIEM/Release 4.12.0 Feb 27, 2025
@wazuhci wazuhci moved this from Pending final review to Done in XDR+SIEM/Release 4.12.0 Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
level/task Task issue type/bug Bug issue
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants