-
-
Notifications
You must be signed in to change notification settings - Fork 22.4k
Android: Add export option for custom theme attributes #106724
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?
Conversation
b63f3c1
to
5d8dc85
Compare
<!-- GodotAppMainTheme is auto-generated during export. Manual changes will be overwritten. | ||
To add custom attributes, use the "gradle_build/custom_theme_attributes" Android export option. --> |
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.
Similar logic needs to be applied to GodotAppSplashTheme
as well. In my PR, the android:windowIsTranslucent
attribute is added to both themes, and updated when transparency is enabled.
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.
To address that, we should be able to duplicate the same set of attributes across both themes. We can have different defaults for each themes, but the custom theme attributes should be applied to both themes.
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.
We can re-generate it during export with only default attributes. GodotAppSplashTheme
only handles the splash screen, I don't see any need for a custom attribute here.
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 would have agreed, except that my testing of the transparency PR showed that I needed to set android:windowIsTranslucent
on both themes for it to take effect.
I suspect that because it's the first theme, the splash theme may set some properties that cannot be updated by the main theme.
And so to ensure that the custom theme attributes don't run into the same problem I did, they should be applied to both themes.
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'm not sure about duplicating all custom theme attributes. Some attributes in GodotAppSplashTheme
might cause unexpected behaviour and user wouldn't have any way to avoid that without core changes or removing that attribute from both themes.
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.
@m4gr3d I introduced a prefix for GodotAppSplashTheme
attributes. If custom_theme_attributes
has a key like [splash]android:windowIsTranslucent
it would be added to the GodotAppSplashTheme
.
8baabfb
to
51c21cd
Compare
- Regenerates the `GodotAppMainTheme` and `GodotAppSplashTheme` during Android export. Any manual changes to these styles will be cleared and replaced with default theme attributes. - Adds a new export option `gradle_build/custom_theme_attributes` for injecting custom theme attributes directly via the export window, avoiding the need to manually modify themes.xml.
51c21cd
to
529e03d
Compare
Regenerate the
GodotAppMainTheme
andGodotAppSplashTheme
during Android export. Any manual changes to this style will be cleared and replaced with default theme attributes.Adds a new export option
gradle_build/custom_theme_attributes
for injecting custom theme attributes directly via the export window, avoiding the need to manually modify themes.xml.It helps to avoid the issue I metioned in #106709 (comment).
According to the attributes shown in the image:
<item name="android:windowIsTranslucent">true</item>
to the GodotAppMainTheme.[splash]
, so it adds the attribute to the GodotAppSplashTheme.