Skip to content

Conversation

@dukke
Copy link
Contributor

@dukke dukke commented Jan 9, 2025

Copy link
Contributor

@dholubek dholubek left a comment

Choose a reason for hiding this comment

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

have a comment about using ubiquitous .icon class here.

* *
******************************************************************************/

.icon {
Copy link
Contributor

Choose a reason for hiding this comment

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

are we sure this won't break anything? do all the icons in Komet have a background color white?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've removed that css definition and moved it to the icon-klcontext-menu styleclass to make sure it only applies to the context menu icons

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some before and after images in the PR?
I feel this fix is only localized, I thought something happened recently when we switched out something. Lot's of other buttons and icons (graphic of button/toggle, svgnode, regions, etc.) in other screens are also squished.

Copy link
Contributor Author

@dukke dukke Jan 9, 2025

Choose a reason for hiding this comment

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

@carldea Images have been attached to the issue

dukke added 3 commits January 9, 2025 15:36
…g white to the icon-klcontext-menu styleclass
# Conflicts:
#	kview/src/main/java/dev/ikm/komet/kview/controls/skin/KLReadOnlyStringControlSkin.java
Copy link
Contributor

@dholubek dholubek left a comment

Choose a reason for hiding this comment

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

approved

@dholubek dholubek requested review from carldea and dholubek January 13, 2025 14:04
Copy link
Contributor

@dholubek dholubek left a comment

Choose a reason for hiding this comment

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

approved

@dholubek dholubek merged commit 3772f44 into ikmdev:main Jan 13, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants