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

Adds multiple icon classes and allows changing the color #1508

Merged
merged 2 commits into from
Feb 27, 2025

Conversation

mko-sci
Copy link
Contributor

@mko-sci mko-sci commented Feb 27, 2025

Description

These icons are used in the new OX-Watchlist feature.

Additional Notes

  • This PR fixes or works on following ticket(s): OX-11901

Checklist

  • Code change has been tested and works locally
  • Code was formatted via IntelliJ and follows SonarLint & best practices

@mko-sci mko-sci added 🧬 Enhancement Contains new features 👶🏻 Trivial Easy to review labels Feb 27, 2025
}

.sci-icon-bars-blue {
background-image: url(svgResource('/assets/images/icons/fontawesome/bars.svg', #3465CC));
Copy link
Member

Choose a reason for hiding this comment

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

I see that we use the explicit color code elsewhere as well – but do we need to? Is there no matching colour in the palette of predefined colours? If not, why do we need particularly that blue here then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thats exactly what I thought, but no - there is no predefined color for the blue - and also when you take a look into the file - there are a lot of custom colors in there already.
I am wondering why we even started with the sirius-colors when no one really cares..

Copy link
Contributor Author

@mko-sci mko-sci Feb 27, 2025

Choose a reason for hiding this comment

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

If not, why do we need particularly that blue here then?

I dont know - its the blue we use in OX within the Datasheet and Infoplay dialogs and other places aswell...

Copy link
Member

Choose a reason for hiding this comment

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

$sirius-primary looks close enough, doesn't it? – if we want to port OX to use that color as well, of course

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup - but I think this should be done in a separate ticket - especially since we would neet to change a lot of places in OX as well

Copy link
Member

Choose a reason for hiding this comment

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

@mko-sci Will you create a ticket for this?

@mko-sci mko-sci merged commit d70ef1a into develop Feb 27, 2025
5 checks passed
@mko-sci mko-sci deleted the feature/mko/OX-11901-2 branch February 27, 2025 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧬 Enhancement Contains new features 👶🏻 Trivial Easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants