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

Update on Add AnkiWeb logo to 'AnkiWeb account' screen #16300 #16672

Closed
wants to merge 9 commits into from

Conversation

tamojitdas
Copy link

Purpose / Description

Add AnkiWeb logo to 'AnkiWeb account' screen #16300

Fixes

Approach

Brings updates to the UI (certain pages) of the application.

How Has This Been Tested?

Tested in local environment through Android Studio Jellyfish | 2023.3.1, Device: Emulated -> Pixel 8 Pro API UpsideDownCakePrivacySandbox, SDK -> Android 14.0 "UpsideDownCake"

Checklist

Please, go through these checks before submitting the PR.

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • UI Changes: You have tested your change using the Google Accessibility Scanner

Copy link

welcome bot commented Jun 28, 2024

First PR! 🚀 We sincerely appreciate that you have taken the time to propose a change to AnkiDroid! Please have patience with us as we are all volunteers - we will get to this as soon as possible.

Copy link
Contributor

Message to maintainers, this PR contains strings changes.

  1. Before merging this PR, it is best to run the "Sync Translations" GitHub action, then make and merge a PR from the i18n_sync branch to get translations cleaned out.
  2. Then merge this PR, and immediately do another translation PR so the huge change made by this PR's key changes are all by themselves.

Read more about updating strings on the wiki,

@tamojitdas
Copy link
Author

hi @snowtimeglass kindly consider this PR for the issue #16300 Update on Add AnkiWeb logo to 'AnkiWeb account' screen.
Apologies for the delay, got engaged in some urgent office activities for a while.

Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! Regarding this pull request:

  • the PR should include ONLY the changes that are needed for the feature you're implementing. Currently, there are a lot of unrelated changes to code and layout formatting, extra imports and not needed comments which makes this PR very hard to review. Please revert them.
  • whenever making UI changes we require some screenshots with the changed UI in both the light and dark theme
  • you used a lot of inconsistent padding/margins values(ex: not using multiple of 4dp, the material standard) and a lot of negative margins which are most likely not needed.

Also, if you can, it would be really useful if you could rebase your changes on top of main so there would be a single commit.

@lukstbit lukstbit added the Needs Author Reply Waiting for a reply from the original author label Jun 29, 2024
Copy link
Contributor

Hello 👋, this PR has had no activity for more than 2 weeks and needs a reply from the author. If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing! You have 7 days until this gets closed automatically

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Author Reply Waiting for a reply from the original author New contributor Stale Strings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[suggestion] Add AnkiWeb logo to 'AnkiWeb account' screen
2 participants