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

CCCT-533- ui icon changes as per the new figma #2892

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

pm-dimagi
Copy link

@pm-dimagi pm-dimagi commented Nov 11, 2024

https://dimagi.atlassian.net/browse/CCCT-533

The ui changes
-- Icon update
-- commcare logo update

Safety Assurance

  • If the PR is high risk, "High Risk" label is set
  • I have confidence that this PR will not introduce a regression for the reasons below
  • Do we need to enhance manual QA test coverage ? If yes, "QA Note" label is set correctly

Automated test coverage

Safety story

Release Note: Updated Iconography and Logo

Copy link
Contributor

@shubham1g5 shubham1g5 left a comment

Choose a reason for hiding this comment

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

The code changes looks fine.

  1. Is it possible to add screenshots in the PR on how it looks in the app ?
  2. Is it possible to use vector drawables insted of png to take into account different screen densities.

@pm-dimagi
Copy link
Author

pm-dimagi commented Nov 12, 2024

WhatsApp Image 2024-11-12 at 11 34 50 (1)
WhatsApp Image 2024-11-12 at 11 34 50 (2)
WhatsApp Image 2024-11-12 at 11 34 50
@avazirna

Copy link
Contributor

@avazirna avazirna left a comment

Choose a reason for hiding this comment

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

@pm-dimagi can you include a screenshot of the full Home screen, with the Incomplete and Saved buttons?

app/res/layout/install_confirm_fragment.xml Outdated Show resolved Hide resolved
@pm-dimagi
Copy link
Author

Screenshot_20241112_154443_CommCare Debug

Copy link
Contributor

@shubham1g5 shubham1g5 left a comment

Choose a reason for hiding this comment

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

Changes looks good to me. It's a good idea to add a release note to any big changes like this and also tick the safety checklist in PR template.

@shubham1g5 shubham1g5 added this to the 2.55 milestone Nov 13, 2024
@avazirna
Copy link
Contributor

@damagatchi retest this please

@kcowger
Copy link

kcowger commented Nov 14, 2024

@pm-dimagi I'm seeing a different design (different icons, different icon sizes) on the final assets provided to SaaS here: https://www.figma.com/design/UWjp1wRNAO0o9q2llwDKMc/CommCare-Cosmetic-Changes-Post-Nov-'22Rebrand?node-id=111-2&node-type=canvas

Can you confirm where you got these icons and icon sizes? I want to make sure we are working on off of the same assets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants