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

SMARTWIFI-10829 tags update #404

Merged
merged 2 commits into from
Jan 14, 2025
Merged

SMARTWIFI-10829 tags update #404

merged 2 commits into from
Jan 14, 2025

Conversation

jeprubio
Copy link
Contributor

@jeprubio jeprubio commented Jan 13, 2025

🥅 What's the goal?

Update tag color tokens and remove inverse.
Discussion about the colors and the inverse removal:
https://teams.microsoft.com/l/message/19:[email protected]/1736771738837?tenantId=9744600e-3e04-492e-baa1-25ec245c6f10&groupId=e265fe99-929f-45d1-8154-699649674a40&parentMessageId=1736771738837&teamName=M%C3%ADstica&channelName=design-appscore&createdTime=1736771738837&ngc=true

🚧 How do we do it?

  • Updated tag tokens following the figma design.
  • Removed inverse type (breaking change, Active should be used instead)
  • Once merged, add the breaking change to the new mistica version 18.0.0

☑️ Checks

  • I updated the documentation, including readmes and wikis. If this is a breaking change, tag the PR with "Breaking Change" label and remember to include breaking change migration guide in release notes where this version is released.
  • Tested with dark mode.
  • Tested with API 24.
  • Sync done with iOS team for this feature to ensure alignment, if applies.
  • Accessibility considerations.

🧪 How can I test this?

If it cannot be tested explain why.

  • 🖼️ Screenshots/Videos
Before After
classic views before Classic views after
Compose Before Compose After
classic views before Dark Mode Classic views after Dark Mode
Compose Before Dark Mode Compose After Dark Mode
  • Mistica App QR or download link (PR comment)
  • Reviewed by Mistica design team (reviewer added)

@jeprubio jeprubio requested review from juangardi21, a team, yamal-alm and marinitx and removed request for a team January 13, 2025 15:38
@jeprubio jeprubio marked this pull request as ready for review January 13, 2025 15:38
Comment on lines -151 to -158
TYPE_PROMO -> with(MisticaTheme.colors) { promoLow to promoHigh }
TYPE_ACTIVE -> with(MisticaTheme.colors) { brandLow to brand }
TYPE_INACTIVE -> with(MisticaTheme.colors) { neutralLow to neutralMedium }
TYPE_SUCCESS -> with(MisticaTheme.colors) { successLow to successHigh }
TYPE_WARNING -> with(MisticaTheme.colors) { warningLow to warningHigh }
TYPE_ERROR -> with(MisticaTheme.colors) { errorLow to errorHigh }
TYPE_INVERSE -> with(MisticaTheme.colors) { inverse to brand }
else -> with(MisticaTheme.colors) { promoLow to promoHigh }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Colour changes for compose

Comment on lines -91 to -98
TYPE_PROMO -> R.attr.colorPromoLow to R.attr.colorPromoHigh
TYPE_ACTIVE -> R.attr.colorBrandLow to R.attr.colorBrand
TYPE_INACTIVE -> R.attr.colorNeutralLow to R.attr.colorNeutralMedium
TYPE_SUCCESS -> R.attr.colorSuccessLow to R.attr.colorSuccessHigh
TYPE_WARNING -> R.attr.colorWarningLow to R.attr.colorWarningHigh
TYPE_ERROR -> R.attr.colorErrorLow to R.attr.colorErrorHigh
TYPE_INVERSE -> R.attr.colorInverse to R.attr.colorBrand
else -> R.attr.colorPromoLow to R.attr.colorPromoHigh
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Colour changes for classic views

@jeprubio jeprubio added the Breaking Change A change that is not retrocompatible label Jan 13, 2025
@yceballost yceballost requested review from yceballost and removed request for marinitx January 13, 2025 16:15
Copy link
Contributor

@juangardi21 juangardi21 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

Copy link
Contributor

@yceballost yceballost left a comment

Choose a reason for hiding this comment

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

great!

I can't see link to check it in Mística Catalog, but can you include screenshots for dark mode in PR description? Everything should fine but just to double check

@jeprubio
Copy link
Contributor Author

jeprubio commented Jan 13, 2025

great!

I can't see link to check it in Mística Catalog, but can you include screenshots for dark mode in PR description? Everything should fine but just to double check

Sure, I've just added the screenshots in dark mode to the PR description.

This github action is failing all the time in uploading the catalog to appcenter https://github.com/Telefonica/mistica-android/actions/runs/12750478199/job/35539118396?pr=404 I'm trying to find out if there is a way for this to be fixed.

Copy link

📱 New catalog for testing generated: Download

Copy link
Contributor

@yamal-alm yamal-alm left a comment

Choose a reason for hiding this comment

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

👍

@jeprubio jeprubio merged commit 6aa7e8c into main Jan 14, 2025
5 checks passed
@jeprubio jeprubio deleted the task/SMARTWIFI-10829-TagsUpdate branch January 14, 2025 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change A change that is not retrocompatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants