-
Notifications
You must be signed in to change notification settings - Fork 187
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
Fix styles in deploy new agent section #4830
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We saw witch @chantal-kelm that the option for Suse 12 has the same problem as before.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.euiButtonGroup__buttons { | ||
display: grid; | ||
grid-template-columns: repeat(3, 1fr); | ||
grid-template-columns: repeat(5, 1fr); | ||
grid-gap: 10px; | ||
padding: 0 3px; | ||
box-shadow: none; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is this a class of the @elastic/eui? If it does, we should ensure only to affect the UI of the Wazuh plugin. This could be modifying the styles of plugins of the platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in this PR
.euiButtonGroup--medium .euiButtonGroupButton:not(:first-child), .euiButtonGroup--small .euiButtonGroupButton:not(:first-child) { | ||
margin-left: 0 !important; | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is this a class of the @elastic/eui? If it does, we should ensure only to affect the UI of the Wazuh plugin. This could be modifying the styles of plugins of the platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: For another hand, this pull request has as its base the branch 4.4-2.3-wzd
, which is a plugin compatible with Wazuh dashboard based on OpenSearch Dashboards 2.3.0. This version of OpenSearch is using a forked UI library, https://github.com/opensearch-project/OpenSearch-Dashboards/blob/2.3.0/package.json#L111, and it seems the class names are prefixed by oui
instead of eui
https://github.com/opensearch-project/oui/blob/1.0.0/src/components/button/button_group/_button_group_button.scss#L122. Is these styles applying?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: For another hand, this pull request has as its base the branch
4.4-2.3-wzd
, which is a plugin compatible with Wazuh dashboard based on OpenSearch Dashboards 2.3.0. This version of OpenSearch is using a forked UI library, https://github.com/opensearch-project/OpenSearch-Dashboards/blob/2.3.0/package.json#L111, and it seems the class names are prefixed byoui
instead ofeui
https://github.com/opensearch-project/oui/blob/1.0.0/src/components/button/button_group/_button_group_button.scss#L122. Is these styles applying?
I was testing the branch and it is working. It seems that the UI library contains components from @elastic/eui and oui components.
Anyways, we should ensure that the modified styles only happen in the Wazuh plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in this PR
display: grid; | ||
grid-template-columns: repeat(5, 1fr); | ||
grid-gap: 10px; | ||
.message { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: this class name is generic and could have conflicts with the styles of other plugins, we should use scoped names, for example prefixing it with wz
, wazuh
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in this PR
Description
In Wazuh info, in the 'Deploy new agent' section, the OS and versions were not displayed correctly.
Evidence
This is what Wazuh Info used to look like:
This is what it looks like now:
Steps to test:
*Go to the Agents tab
*Click on the 'Deploy new agent' button.
*Verify that the styles are displayed correctly.