-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[JEWEL-954] Implement ad text in popups #3358
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
base: master
Are you sure you want to change the base?
[JEWEL-954] Implement ad text in popups #3358
Conversation
platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/PopupContainer.kt
Outdated
Show resolved
Hide resolved
b0a454a to
9ecf162
Compare
faogustavo
left a comment
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.
I think that you are missing the LaF styles loading. Also, make sure to get values to match the theme.
For more details in the AdText styles, check the JBUI.CurrentTheme.Advertiser type. You can find its usage to use as baseline on this function public void setAdText(@NotNull @NlsContexts.PopupAdvertisement String s, int alignment)
4356525 to
c4d0b61
Compare
...el/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/theme/IntUiBridgePopupAdText.kt
Outdated
Show resolved
Hide resolved
...el/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/theme/IntUiBridgePopupAdText.kt
Outdated
Show resolved
Hide resolved
c4d0b61 to
91fba7f
Compare
ecf2915 to
20db336
Compare
faogustavo
left a comment
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.
Don't forget to update API Dumps after updating public apis. Run the 'apiChecks' task in the IDE for it :)
...lone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/styling/IntUiPopupAdTextStyling.kt
Outdated
Show resolved
Hide resolved
...i/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/theme/IntUiTheme.kt
Show resolved
Hide resolved
20db336 to
259bd6b
Compare
...i/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/theme/IntUiTheme.kt
Show resolved
Hide resolved
platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ComboBox.kt
Show resolved
Hide resolved
platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/ListComboBox.kt
Show resolved
Hide resolved
platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/PopupContainer.kt
Show resolved
Hide resolved
platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/DefaultComponentStyling.kt
Show resolved
Hide resolved
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.
For references, this is a sample of breaking change. Note that the method signature changed (even when you've added a default parameter). For the JVM, that's a breaking change and will cause runtime issues.
When you keep the old method with "Deprecation Level Hidden", the old method becomes "synthetic" (therefore get's hidden from normal usages), but is still available for old compilations using that method.
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.
Complementing this info: to ensure the deprecation flow was correctly applied, check the changes in the platform/jewel/int-ui/int-ui-standalone/api-dump.txt file
- sf:dark(->- bsf:dark(
The only change was the addition of the b. The parameters remained the same.
Also, here is a list with all modifiers:
* - marked with @ApiStatus.Experimental.
b - synthetic (as in binary visible). Members with both ACC_SYNTHETIC and ACC_BRIDGE are not included in the dump.
p - protected. Nothing for public because it's the default. private and package-private members are not included.
s - static.
@ - annotation.
e - enum.
f - final.
a - abstract.
c - class. Nothing for interface because it should be the default.
259bd6b to
779db91
Compare
779db91 to
60a6e3f
Compare
faogustavo
left a comment
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.
👏
Summary
This PR introduces a new component called
PopupAdText, which is designed to be used inside a popup container to give a more informative and/or helpful description to that menu. To apply that, a new parameter calledadTextwas introduced to all public API of comboboxes, so no need to setup layout, spacing and stuff like that. This API is designed to be simpler as we have inAbstractPopup#setAdTextmethod in the legacy Swing implementation.Release notes
PopupAdTextStyleto theJewelThemeand to theIntUiThemePopupAdTextcomponentNew features
Note
Adds optional ad text to popups and combo boxes with full theming support.
PopupAdTextcomponent andPopupAdTextStyle(colors/metrics/textStyle) withLocalPopupAdTextStyleandJewelTheme.popupAdTextStylePopupContainernow renders bottomadText;ComboBox,ListComboBox, andEditableListComboBoxAPIs gain optionaladTextplumbed through implementationsDefaultComponentStylingextended withpopupAdTextStyle;IntUiTheme(light/dark) and Swing bridge provide style (readPopupAdTextStyle, standalonelight()/dark())Written by Cursor Bugbot for commit 91fba7f. This will update automatically on new commits. Configure here.