-
Notifications
You must be signed in to change notification settings - Fork 42
SES-4569 : 'Follow Setting' button doesn't follow theming #1531
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
SES-4569 : 'Follow Setting' button doesn't follow theming #1531
Conversation
binding.followSetting.isGone = true | ||
|
||
val isLight = ThemeUtil.isLightTheme(context) | ||
if(isLight){ |
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 we might also need to handle the "else" case so that if reusing the view doesn't cause issue
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.
Given you are using attribute, maybe this color can just apply using attribute in the XML regardless?
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.
Yup, I'll be adding this to the XML themes and apply it directly to the textview.
if(isLight){ | ||
binding.followSetting.setTextColor(context.getColorFromAttr(android.R.attr.textColorPrimary)) | ||
} | ||
|
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.
In Compose we already have this setup in our theme as accentText
, which sets the color to accent on dark themes and text on light themes.
That color isn't defined in the xml theme though, so you can either go with what you wrote here since hopefully we should only be dealing with Compose for new elements, otherwise you can also define accentText
as an attributein xml and set it for each 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 think adding this to the themes is better. Will make the change, thanks!
* Use textColorPrimary for light theme for Follow Setting button/text * Added to accentText to themes * Added accentText for the test theme --------- Co-authored-by: ThomasSession <thomas.r@getsession.org>
* ADded data for non originating subscriptions * Tweaking state to remove labels from VM state to help with all the screen variations we have * WIP * BAse Non Originating screen * Fixing broken strings * Non originating UI * Non originating UI * Bump org.mockito:mockito-core from 5.19.0 to 5.20.0 (#1533) * Initial implementation of google play billing (#1503) * [Automated] Update translations from Crowdin * Bump io.github.webrtc-sdk:android from 137.7151.01 to 137.7151.04 (#1513) Bumps [io.github.webrtc-sdk:android](https://github.com/webrtc-sdk/android) from 137.7151.01 to 137.7151.04. - [Release notes](https://github.com/webrtc-sdk/android/releases) - [Changelog](https://github.com/webrtc-sdk/android/blob/v137.7151.04/CHANGES.md) - [Commits](webrtc-sdk/android@v137.7151.01...v137.7151.04) --- updated-dependencies: - dependency-name: io.github.webrtc-sdk:android dependency-version: 137.7151.04 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: SessionHero01 <180888785+SessionHero01@users.noreply.github.com> * Bump cameraCamera2Version from 1.4.2 to 1.5.0 (#1514) Bumps `cameraCamera2Version` from 1.4.2 to 1.5.0. Updates `androidx.camera:camera-camera2` from 1.4.2 to 1.5.0 Updates `androidx.camera:camera-lifecycle` from 1.4.2 to 1.5.0 Updates `androidx.camera:camera-view` from 1.4.2 to 1.5.0 --- updated-dependencies: - dependency-name: androidx.camera:camera-camera2 dependency-version: 1.5.0 dependency-type: direct:production update-type: version-update:semver-minor - dependency-name: androidx.camera:camera-lifecycle dependency-version: 1.5.0 dependency-type: direct:production update-type: version-update:semver-minor - dependency-name: androidx.camera:camera-view dependency-version: 1.5.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: SessionHero01 <180888785+SessionHero01@users.noreply.github.com> * Bump androidxHiltVersion from 1.2.0 to 1.3.0 (#1532) Bumps `androidxHiltVersion` from 1.2.0 to 1.3.0. Updates `androidx.hilt:hilt-navigation-compose` from 1.2.0 to 1.3.0 Updates `androidx.hilt:hilt-work` from 1.2.0 to 1.3.0 Updates `androidx.hilt:hilt-compiler` from 1.2.0 to 1.3.0 --- updated-dependencies: - dependency-name: androidx.hilt:hilt-navigation-compose dependency-version: 1.3.0 dependency-type: direct:production update-type: version-update:semver-minor - dependency-name: androidx.hilt:hilt-work dependency-version: 1.3.0 dependency-type: direct:production update-type: version-update:semver-minor - dependency-name: androidx.hilt:hilt-compiler dependency-version: 1.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: SessionHero01 <180888785+SessionHero01@users.noreply.github.com> * [Automated] Update translations from Crowdin * Bump com.google.gms.google-services from 4.4.2 to 4.4.3 (#1534) Bumps com.google.gms.google-services from 4.4.2 to 4.4.3. --- updated-dependencies: - dependency-name: com.google.gms.google-services dependency-version: 4.4.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: SessionHero01 <180888785+SessionHero01@users.noreply.github.com> * Temp string replacement * Updated full perission check, added function for adding more media * [Automated] Update translations from Crowdin * Added plus menu to add to allowed media * udpated condition * removed deprecated method * SES-4569 : 'Follow Setting' button doesn't follow theming (#1531) * Use textColorPrimary for light theme for Follow Setting button/text * Added to accentText to themes * Added accentText for the test theme --------- Co-authored-by: ThomasSession <thomas.r@getsession.org> * Fixed menu button redrawing --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: ThomasSession <thomas.r@getsession.org> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: SessionHero01 <180888785+SessionHero01@users.noreply.github.com> Co-authored-by: ThomasSession <171472362+ThomasSession@users.noreply.github.com> Co-authored-by: Bilb <1544279+Bilb@users.noreply.github.com>
SES-4569
This PR adds a condition to use the primary text color for the "Follow Setting" text when in light theme and use the accent color instead for dark theme.