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

Add icons for Dashboards 2.7 #658

Merged
merged 6 commits into from
Apr 7, 2023
Merged

Conversation

BSFishy
Copy link
Contributor

@BSFishy BSFishy commented Apr 4, 2023

Description

Add the icons that are needed for Dashboards 2.7. A few items of cleanup needed to happen. Namely:

  1. Add SPDX header
  2. Indent paths
  3. Remove fills on paths
  4. Re-order svg tag
  5. Remove fill on svg tag

2023-04-03 17_36_35-Icons - OpenSearch UI Framework
2023-04-03 17_36_42-Icons - OpenSearch UI Framework
2023-04-05 14_55_46-Icons - OpenSearch UI Framework
2023-04-05 14_55_52-Icons - OpenSearch UI Framework

Issues Resolved

Closes #628

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • All tests pass
    • yarn lint
    • yarn test-unit
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Matt Provost <provomat@amazon.com>
@BSFishy BSFishy requested a review from a team as a code owner April 4, 2023 00:46
@KrooshalUX
Copy link
Contributor

Can you attach a screenshot of the docs site with the icons added for UX sign off? :D

@BSFishy
Copy link
Contributor Author

BSFishy commented Apr 4, 2023

Can you attach a screenshot of the docs site with the icons added for UX sign off? :D

I took screenshots but forgot to attach them :P updated the PR 👍

@kgcreative
Copy link
Member

I'm wondering if instead of visBuilderPPL we should say visPPL?

@BSFishy
Copy link
Contributor Author

BSFishy commented Apr 5, 2023

I'm wondering if instead of visBuilderPPL we should say visPPL?

I mainly just copied the naming convention of the files that were supplied. Can change any names if needed 👍

@kgcreative
Copy link
Member

@KrooshalUX - can you comment on naming?

@KrooshalUX
Copy link
Contributor

KrooshalUX commented Apr 5, 2023

@kgcreative Since that structure was already being used for visType, I wanted to give it another name category, since these are "ways to build a visualization" - an issue regarding clarity that we are battling in multiple places right now. This naming convention also helps keep them grouped together on the OUI Icons documentation page , vs scattered between chart types - is already fairly hard to scan through the icon page.

However, if you feel strongly about removing the "builder" part from here, that is fine - I don't really feel that strongly about it.

@kgcreative
Copy link
Member

I think my main concern on naming is making sure we weren't conflating this with the visBuilder project icons, since these are primarily there to help support the observability visualizations. I'm open to other names.

@KrooshalUX
Copy link
Contributor

KrooshalUX commented Apr 5, 2023

Please leave visBuilder, and visBuilderSavedObject as is (note: I wouldnt normally call out SavedObject in an icon name, but we needed a level of clarity for this round and use in the icon set. I was recommending a 1:1 use of the icon to the saved object, but in case that is not possible, we created a fall back. I would prefer to clean this up during our Data Explorer project)

Please change:
visBuilderDQL to visQueryDQL (this probably wont get used, but it was supplied in case)
visBuilderPPL to visQueryPPL
visBuilderSQL to visQuerySQL
visBuilderPromQL to visQueryPromQL

@KrooshalUX KrooshalUX mentioned this pull request Apr 5, 2023
8 tasks
seanneumann and others added 3 commits April 5, 2023 13:18
Signed-off-by: Matt Provost <provomat@amazon.com>
Signed-off-by: Matt Provost <provomat@amazon.com>
Copy link
Collaborator

Choose a reason for hiding this comment

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

We will revisit this later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For context: this svg file is 32x33 rather than 32x32. We're trying to push this out for 2.7, so we'll fix this in a later update and just focus on getting this PR through right now

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an issue tracking?

Copy link
Contributor

Choose a reason for hiding this comment

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

Strange, sorry about that. I will correct and re-export when I am back at my desk.

Signed-off-by: Matt Provost <provomat@amazon.com>
@BSFishy
Copy link
Contributor Author

BSFishy commented Apr 7, 2023

@kgcreative need final UX review

Copy link
Contributor

@KrooshalUX KrooshalUX left a comment

Choose a reason for hiding this comment

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

LGTM

@BSFishy BSFishy merged commit 9e18194 into opensearch-project:main Apr 7, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 7, 2023
* Add icons

Signed-off-by: Matt Provost <provomat@amazon.com>

* Rename icons to correct name

Signed-off-by: Matt Provost <provomat@amazon.com>

* Update svg copyright header

Signed-off-by: Matt Provost <provomat@amazon.com>

---------

Signed-off-by: Matt Provost <provomat@amazon.com>
Co-authored-by: Sean Neumann <1413295+seanneumann@users.noreply.github.com>
(cherry picked from commit 9e18194)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 7, 2023
* Add icons

Signed-off-by: Matt Provost <provomat@amazon.com>

* Rename icons to correct name

Signed-off-by: Matt Provost <provomat@amazon.com>

* Update svg copyright header

Signed-off-by: Matt Provost <provomat@amazon.com>

---------

Signed-off-by: Matt Provost <provomat@amazon.com>
Co-authored-by: Sean Neumann <1413295+seanneumann@users.noreply.github.com>
(cherry picked from commit 9e18194)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
joshuarrrr pushed a commit that referenced this pull request Apr 12, 2023
* Add icon
* Rename icons to correct name
* Update svg copyright header

---------



(cherry picked from commit 9e18194)

Signed-off-by: Matt Provost <provomat@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Sean Neumann <1413295+seanneumann@users.noreply.github.com>
joshuarrrr pushed a commit that referenced this pull request Apr 12, 2023
* Add icons
* Rename icons to correct name
* Update svg copyright header

---------



(cherry picked from commit 9e18194)

Signed-off-by: Matt Provost <provomat@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Sean Neumann <1413295+seanneumann@users.noreply.github.com>
@joshuarrrr joshuarrrr mentioned this pull request Sep 1, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add icons to icon pack
5 participants