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

[TSVB] Remove font awesome icons from annotations and use EUI instead #167949

Closed
stratoula opened this issue Oct 4, 2023 · 1 comment · Fixed by #172661
Closed

[TSVB] Remove font awesome icons from annotations and use EUI instead #167949

stratoula opened this issue Oct 4, 2023 · 1 comment · Fixed by #172661
Assignees
Labels
Feature:TSVB TSVB (Time Series Visual Builder) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture

Comments

@stratoula
Copy link
Contributor

Describe the feature:

TSVB annotations use src/plugins/vis_types/timeseries/public/application/components/icon_select/icon_select.js icons on the icons dropdown which exists on annotations. We should use EUI instead as we do in Lens.
image

There is also one icon here src/plugins/vis_types/timeseries/public/application/components/vis_editor_visualization.js but it is not visible so

we have 2 options:

  1. Remove it (or replace with EUI but it is not visible as I said above)
  2. Use the EUI resizable component instead of the custom button
<button
            className="tvbEditorVisualization__draghandle"
            onMouseDown={this.handleMouseDown}
            onMouseUp={this.handleMouseUp}
            onKeyDown={this.onSizeHandleKeyDown}
            aria-label={this.props.intl.formatMessage({
              id: 'visTypeTimeseries.colorRules.adjustChartSizeAriaLabel',
              defaultMessage: 'Press up/down to adjust the chart size',
            })}
          >
            <i className="fa fa-ellipsis-h" />
          </button>

This will enable us to remove the package.json/font-awesome

@stratoula stratoula added Feature:TSVB TSVB (Time Series Visual Builder) technical debt Improvement of the software architecture and operational architecture Team:Visualizations Visualization editors, elastic-charts and infrastructure impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. labels Oct 4, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations (Team:Visualizations)

@dej611 dej611 self-assigned this Dec 6, 2023
dej611 added a commit that referenced this issue Dec 6, 2023
## Summary

Fixes #167949 

Replaces missing font-awesome icon with the correct EUI one:

<img width="890" alt="Screenshot 2023-12-06 at 11 12 20"
src="https://github.com/elastic/kibana/assets/924948/4167c4e6-c8b3-4952-8f87-2874008a1f00">

As for the other icons in
`src/plugins/vis_types/timeseries/public/application/components/icon_select/icon_select.js`
they are correctly mapped into EUI ones via these lines:
https://github.com/dej611/kibana/blob/594337a099a68326c9b62149d17a52ad44402268/src/plugins/vis_types/timeseries/public/application/components/icon_select/icon_select.js#L87-L90

As of removing completely `font-awesome` I can still see some usage in
`x-pack/plugins/monitoring/public/components/sparkline/index.js` despite
not having the css imported (so I suspect those icons are actually not
rendered cc @elastic/obs-ux-infra_services-team )
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Visualizations Visualization editors, elastic-charts and infrastructure technical debt Improvement of the software architecture and operational architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants