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

feat: Problem with course menu position on iPad and in Landscape mode on iPhone #292

Merged
merged 53 commits into from
Mar 21, 2024

Conversation

forgotvas
Copy link
Contributor

@forgotvas forgotvas commented Feb 27, 2024

[iOS] Problem with course menu position on iPad and in Landscape mode on iPhone #234

During that ticket was added accessibility injection for increase font size inside webview that depends on accessibility font size.
Paddings based on readability content size that will increase if user will reduce font size and will reduce if user will increase font size.
Demo video

Portrait Changes
Before After
before01 after01
before02 after02
before03 after03
before04 after04
before05 after05
before06 after06
before07 after07
before08 after08
before09 after09
before10 after10
before11 after11
before12 after12
before13 after13
before14 after14
before15 after15
before16 after16
before17 after17
before18 after18
before19 after19
before20 after20
before21 after21
before22 after22
before23 after23
before24 after24
before25 after25
Landscape Changes
Before After
beforeLandscape01 afterLandscape01
beforeLandscape02 afterLandscape02
beforeLandscape03 afterLandscape03
beforeLandscape04 afterLandscape04
beforeLandscape05 afterLandscape05
beforeLandscape06 afterLandscape06
beforeLandscape07 afterLandscape07
beforeLandscape08 afterLandscape08
beforeLandscape09 afterLandscape09
beforeLandscape10 afterLandscape10
beforeLandscape11 afterLandscape11
beforeLandscape12 afterLandscape12
beforeLandscape13 afterLandscape13
beforeLandscape14 afterLandscape14
beforeLandscape15 afterLandscape15
beforeLandscape16 afterLandscape16
beforeLandscape17 afterLandscape17
beforeLandscape18 afterLandscape18
beforeLandscape19 afterLandscape19
beforeLandscape20 afterLandscape20
beforeLandscape21 afterLandscape21
beforeLandscape22 afterLandscape22
beforeLandscape23 afterLandscape23
beforeLandscape24 afterLandscape24

@forgotvas forgotvas marked this pull request as ready for review February 27, 2024 13:13
@forgotvas forgotvas changed the title Problem with course menu position on iPad and in Landscape mode on iPhone feat: Problem with course menu position on iPad and in Landscape mode on iPhone Feb 27, 2024
@forgotvas forgotvas marked this pull request as draft February 28, 2024 08:34
rnr
rnr previously approved these changes Mar 20, 2024
@rnr
Copy link
Contributor

rnr commented Mar 20, 2024

@saeedbashir We are all just waiting for your review. Please take a look. Thank you.

@saeedbashir
Copy link
Contributor

  1. @forgotvas Could you please correct the alignment of Or sign in with and Or register with on sign-in and register screens?
  2. I'm not sure how the is working. Although it's look good, but there is enough content in case of Privacy Policy, etc and it left space empty space on both sides. The same is the case with HTML based blocks. But the discover and program web views uses full available space.

image
image

@saeedbashir
Copy link
Contributor

Not sure, but we might have two course cards in one row in case of iPad, just like we have it on production app.
image

@forgotvas
Copy link
Contributor Author

Not sure, but we might have two course cards in one row in case of iPad, just like we have it on production app. image

@rnr ^^^

@forgotvas
Copy link
Contributor Author

  1. @forgotvas Could you please correct the alignment of Or sign in with and Or register with on sign-in and register screens?
  2. I'm not sure how the is working. Although it's look good, but there is enough content in case of Privacy Policy, etc and it left space empty space on both sides. The same is the case with HTML based blocks. But the discover and program web views uses full available space.

image image

Hi @saeedbashir,

  1. Do you want it to be centered?
  2. In iOS 9 apple introduced readabilityContentGuide https://developer.apple.com/documentation/uikit/uiview/1622644-readablecontentguide#discussion and dynamic type. They made that release to make user easily be read without forcing users to move their head to track the lines. Some old tutorial.
    In guidelines apple wrote:

Modify the built-in text styles if necessary. System APIs define font adjustments — called symbolic traits — that let you modify some aspects of a text style. For example, the bold trait adds weight to text, letting you create another level of hierarchy. You can also use symbolic traits to adjust leading if you need to improve readability or conserve space. For example, when you display text in wide columns or long passages, more space between lines (loose leading) can make it easier for people to keep their place while moving from one line to the next. Conversely, if you need to display multiple lines of text in an area where height is constrained — for example, in a list row — decreasing the space between lines (tight leading) can help the text fit well. If you need to display three or more lines of text, avoid tight leading even in areas where height is limited. For developer guidance, see leading(_:).

We didn't touch discovery and programs tabs because there not a content type it's more like browser so it should be on full screen.

@saeedbashir
Copy link
Contributor

  1. Do you want it to be centered?

How about aligning it to other content like textfields?

@rnr
Copy link
Contributor

rnr commented Mar 20, 2024

Not sure, but we might have two course cards in one row in case of iPad, just like we have it on production app. image

@rnr ^^^

@forgotvas I think we could try this approach. Thank you

@forgotvas forgotvas dismissed stale reviews from rnr and volodymyr-chekyrta via e0d5347 March 20, 2024 15:18
@forgotvas
Copy link
Contributor Author

forgotvas commented Mar 20, 2024

  1. Do you want it to be centered?

How about aligning it to other content like textfields?

@saeedbashir, done:

Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2024-03-20 at 18 16 50 Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2024-03-20 at 18 16 54

@forgotvas
Copy link
Contributor Author

Not sure, but we might have two course cards in one row in case of iPad, just like we have it on production app. image

@rnr ^^^

@forgotvas I think we could try this approach. Thank you

@sergeymomot please create ticket for that

saeedbashir
saeedbashir previously approved these changes Mar 21, 2024
@volodymyr-chekyrta
Copy link
Contributor

@forgotvas please resolve the conflicts

@forgotvas
Copy link
Contributor Author

@volodymyr-chekyrta, done

@volodymyr-chekyrta volodymyr-chekyrta merged commit 2de1bc5 into openedx:develop Mar 21, 2024
3 checks passed
@rnr rnr deleted the feat/ipad-stretch branch March 21, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Problem with course menu position on iPad and in Landscape mode on iPhone
4 participants