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

Fix: UI - AnkiWeb account Login & Logout screen UI fixes #17320

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

rohegde7
Copy link

Purpose / Description

UI fix on the Ankiweb account Login and Logout.

Fixes

Approach

  • For Login screen added a horizontal padding to the "Note: AnkiWeb is not affiliated with AnkiDroid" text to avoid the last letter getting truncated
  • For Logout screen I fixed the issue where "Logged in as" text was overlapping with the toolbar. I changed the nested Relative and Linear layout file with a better Constraint layout. Also have used NestedScrollView for any unconventional device layout that might need scrolling. I have made sure to keep all the layout IDs in use so nothing crashes.

How Has This Been Tested?

  • Used physical pixel 4a, Pixel 6a and OnePlus 8 devices to test it manually.

Learning

Constraint Layout:
https://developer.android.com/reference/androidx/constraintlayout/widget/ConstraintLayout

Checklist

  • 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

Using constraint layout for better UI fixed the layers overlapping issue along with the 'd' letter of the text being truncated.
Improvised to retain the previous UI setting with minor improvement in margins of the views
Copy link

welcome bot commented Oct 30, 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.

@mikehardy
Copy link
Member

mikehardy commented Oct 30, 2024

@rohegde7 I truly appreciate the effort here but the issue you filed this PR on already had direct interest from another developer who actively mentioned to you they were working on it.

It is considered impolite and a potential waste of effort via duplication to work on PRs that are claimed when the other person is still expressing active interest.

We may review and merge this PR but I want to be clear that coordination failures like that will not be rewarded.

This PR is not eligible for invoicing on Open Collective. Please ignore comments on this PR from our expense bot.

@rohegde7
Copy link
Author

@mikehardy Thanks for the reply! I didn’t know about these standards. Will follow there things in the future.

Meanwhile may I know what’s the time span devs usually take from commenting about taking on an issue and raising a PR? Do we have a hard rule about it?

Also do we add the dev working on an issue as an assignee?

All help is really appreciated! Excited to be contributing here.

@mikehardy
Copy link
Member

@mikehardy Thanks for the reply! I didn’t know about these standards. Will follow there things in the future.

Excellent - thanks!

Meanwhile may I know what’s the time span devs usually take from commenting about taking on an issue and raising a PR? Do we have a hard rule about it?

There is no rule for it. It is collaborative, the idea is you picture you are in a room with your fellow developers all sitting near each other. Would you just start working on a task that someone else mentioned they were working on? No, of course not, that would be rude.

You would ask. And you would wait and listen for the answer. If there is a truly long period of time between "hey, are you still working on this?" and no answer, say 3-4 days, then go ahead and post again "did not hear back, I'll start on this"

Collaboration is everything

Also do we add the dev working on an issue as an assignee?

In general no, we tried doing that before but it reflects too much ownership, for those rare cases when the dev that expressed initial interest simply does not get back to it, and also it's a burden for maintainers to assign and unassign people. So we keep it slightly less formal than full assignment

All help is really appreciated! Excited to be contributing here.

Nice to have a new dev excited to be here 😄 - welcome!

Copy link
Contributor

@criticalAY criticalAY left a comment

Choose a reason for hiding this comment

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

Root layout should be kept Coordinator layout since snackbars need that /though this activity doesn't have any but as convention/ rest looks good structure wise

@rohegde7
Copy link
Author

rohegde7 commented Nov 12, 2024

Root layout should be kept Coordinator layout since snackbars need that /though this activity doesn't have any but as convention/ rest looks good structure wise

@criticalAY So is it okay if we do not use coordinator here?

@mikehardy
Copy link
Member

@rohegde7 unless there is a technical reason not to do coordinator layout, I believe the answer is "no"
If there is a technical reason, than the way to have progress here is to describe the technical reason
My take on your comment is sort of a "okay but can I ignore this comment?", and... no?

@criticalAY
Copy link
Contributor

I am sure it can work with coordinator layout, and we should follow the convention

@BrayanDSO BrayanDSO added the Needs Author Reply Waiting for a reply from the original author label Nov 14, 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

@github-actions github-actions bot added the Stale label Nov 28, 2024
@rohegde7
Copy link
Author

rohegde7 commented Dec 1, 2024

@criticalAY Sure let me make that change and push the update ASAP!

@github-actions github-actions bot removed the Stale label Dec 1, 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

@github-actions github-actions bot added the Stale label Dec 15, 2024
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 Needs Review New contributor Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AnkiWeb Login Screen
4 participants