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

Mobile: Log in + Thank you screens #226

Closed
marko-srb opened this issue Oct 5, 2023 · 7 comments · Fixed by #240
Closed

Mobile: Log in + Thank you screens #226

marko-srb opened this issue Oct 5, 2023 · 7 comments · Fixed by #240
Assignees
Labels
[Component] Theme Templates, patterns, CSS [Type] Bug Something isn't working
Milestone

Comments

@marko-srb
Copy link

Can we update these two screens on mobile.

login-thanks-mobile-screens

I don't think we need to do anything else then expanding the section itself a bit, and move whole paragraph element more below, plus edit rows for copy — as in screenshot.

Let me know if any help is needed.

@ndiego
Copy link
Member

ndiego commented Oct 6, 2023

This is what I am seeing on the live site. Did you want to minimize the space between the dots and the text?

image

@marko-srb
Copy link
Author

Oh, then it must be some bug to my phone maybe.

When I posted the issue, dots were over the title, apologies for not screenshoting it. At the moment, they are very very close to the title. See screenshot.

IMG_2255

I don't need more space than what you have in the screenshot @ndiego , its just the matter if it looks the same as in your screenshot, or as in mine here on most mobile devices. Let me check on a few more phones, once I have a chance.

Thanks.

@marko-srb marko-srb added the [Status] Needs Design Feedback Feedback is needed on an existing or new component label Oct 10, 2023
@renintw
Copy link
Contributor

renintw commented Oct 10, 2023

Screen.Capture.on.2023-10-11.at.00-47-45.mov

If the window width is reduced on a desktop, at most, the text gets very close to the dots.
However, when switching to different mobile devices, due to the reduced height, overlapping issues occur like on the iPhone SE (as seen at the end of the video).

@ndiego
Copy link
Member

ndiego commented Oct 10, 2023

I think there is some extra margin-bottom on the dots spacer that can be removed. If you remove it, all looks good:
image

@renintw
Copy link
Contributor

renintw commented Oct 10, 2023

Yes, that's correct. I just did a quick test and it seems to resolve the issue here.
But I might need to look into it a bit to understand why this CSS was added in the first place, to ensure that the modification won't break anything elsewhere.

@renintw renintw self-assigned this Oct 10, 2023
@renintw renintw moved this to 🏗 In progress in WordPress.org Oct 10, 2023
@renintw renintw added [Component] Theme Templates, patterns, CSS [Type] Bug Something isn't working labels Oct 10, 2023
@ndiego
Copy link
Member

ndiego commented Oct 10, 2023

But I might need to look into it a bit to understand why this CSS was added in the first place, to ensure that the modification won't break anything elsewhere.

Originally, the design had the text overlaying the dots. That's my guess.

@marko-srb
Copy link
Author

Screen.Capture.on.2023-10-11.at.00-47-45.mov
If the window width is reduced on a desktop, at most, the text gets very close to the dots. However, when switching to different mobile devices, due to the reduced height, overlapping issues occur like on the iPhone SE (as seen at the end of the video).

In my experience, narrowing down the browser size, or Inspect > Mobile, isn't as precise as testing it on the phone itself in most cases. For example, the screenshot I provided is Iphone 13 Pro, it has much larger screen than SE, and still has narrow spacing there.

Originally, the design had the text overlaying the dots. That's my guess.
Yes. But we gave up on that approach, and I assume that wasn't fully fixed on mobile in the meantime.


Spacing 30px, or margin-bottom is system spacing we have under each element on mobile design. On top and bottom its 40px, between all elements is 30px. I think that should stay there. :)

— 

Overall, I am not sure my comments are helpful, so to conclude:
Ideal behavior (per latest design edits/feedback requests) is to have a nice space between the dots and the title, if that artificially means adding more padding for that purpose (like 10-20 px), lets do it. The example is first screenshot I added in this issue on blue background (it is currently 40px space between the dots and the title, could be 30px as well)...

Let me know if this helps, or if you have any other questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Component] Theme Templates, patterns, CSS [Type] Bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants