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: SE device UI issues #39

Merged
merged 15 commits into from
Jul 26, 2024
Merged

fix: SE device UI issues #39

merged 15 commits into from
Jul 26, 2024

Conversation

forgotvas
Copy link
Collaborator

@forgotvas forgotvas commented Jul 24, 2024

Related to iOS Buggy UI openedx#511 including header view for iOS 15

Contains fixes for:

  • Learn Tab > Upgrade to access more features button has icon missing
  • Course Home > UI does not load properly. The course header is covering the content.
  • All Course > Buggy UI
  • Course dashboard > UI does not load properly. The course header is covering the content.
  • Course Dashboard > Expired course’s image disappears after a blink

@forgotvas forgotvas requested review from rnr and saeedbashir July 24, 2024 09:42
Copy link

@saeedbashir saeedbashir left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@forgotvas forgotvas requested a review from saeedbashir July 24, 2024 14:45
@forgotvas
Copy link
Collaborator Author

fixed header for iOS 15

@forgotvas
Copy link
Collaborator Author

Added some fixes for Bug Bash, description updated

@saeedbashir
Copy link

  • The Shift Due Date is slightly under the header, on all devices
  • On rotation the UI disturbs and shows UI of two tabs (only on iPhone 7 iOS 15)

Simulator Screenshot - iPhone 7 - 2024-07-25 at 08 43 45

Screen.Recording.2024-07-25.at.10.39.10.AM.mov

@saeedbashir
Copy link

@forgotvas Most of the iPad issues are fixed in this PR, but the value prop view is not rendered properly.
Simulator Screenshot - iPad (10th generation) - 2024-07-25 at 11 59 57

@forgotvas
Copy link
Collaborator Author

Hi @saeedbashir, i have fixed iPad and fixed Due Date top padding, for rotation bug please add note to Bug Bash. Thank you

@forgotvas
Copy link
Collaborator Author

@saeedbashir fixed rotation bug here too

@saeedbashir
Copy link

saeedbashir commented Jul 26, 2024

@forgotvas Scroll is disabled on Course outline (Home) tab.
https://github.com/user-attachments/assets/bebaa3da-230c-4338-b04e-86819bdccd27

@forgotvas
Copy link
Collaborator Author

@forgotvas Scroll is disabled on Course outline (Home) tab. https://github.com/user-attachments/assets/bebaa3da-230c-4338-b04e-86819bdccd27

fixed

@@ -316,7 +316,7 @@ public struct CourseContainerView: View {
}
}
.versionedTabStyle()
.introspect(.scrollView, on: .iOS(.v15, .v16, .v17), customize: { tabView in
.introspect(.scrollView, on: .iOS(.v16, .v17), customize: { tabView in

Choose a reason for hiding this comment

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

@forgotvas What will happen when the new iOS version iOS 18, will be released, do we need to white list future updates or will it be handled automatically?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we will need add new version or we will need to change standard introspect to @_spi(Advanced) import SwiftUIIntrospect and use it like .introspect(.scrollView, on: .iOS(.v15...), customize: { scrollView in

Choose a reason for hiding this comment

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

Can you update it so we don't have to worry about future updates, until unless there is an issue?

Choose a reason for hiding this comment

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

or if it's out of the scope of this PR, we can create a task for this and can handle it post-MVP.

Copy link
Collaborator Author

@forgotvas forgotvas Jul 26, 2024

Choose a reason for hiding this comment

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

updated, also checked via search on project word .introspect and found that there no any other places where it uses list like .v(16), .v(17). Other places use certain version or relative range.

Copy link

@saeedbashir saeedbashir left a comment

Choose a reason for hiding this comment

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

LGTM 👍

{
"images" : [
{
"filename" : "Regular-M.pdf",

Choose a reason for hiding this comment

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

Could you please change the file name to the asset name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@forgotvas forgotvas merged commit 1dedd69 into 2U/develop Jul 26, 2024
3 checks passed
@forgotvas forgotvas deleted the fix/se branch July 29, 2024 08:02
rnr pushed a commit that referenced this pull request Nov 6, 2024
* fix: se device

* fix: course image

* fix: small course

* fix: header for ios 15

* fix: double nav bar

* chore: added trophy assets

* fix: fixed all courses screen

* fix: ipad course dashboard

* chore: fixed dates banner padding

* fix: ipad upgrade view

* chore: fix for rotation bug

* fix: scroll on home tab

* chore: update introspect for future versions

* chore: rename name of file icon
rnr added a commit that referenced this pull request Nov 7, 2024
* fix: SE device UI issues (#39)

* fix: se device

* fix: course image

* fix: small course

* fix: header for ios 15

* fix: double nav bar

* chore: added trophy assets

* fix: fixed all courses screen

* fix: ipad course dashboard

* chore: fixed dates banner padding

* fix: ipad upgrade view

* chore: fix for rotation bug

* fix: scroll on home tab

* chore: update introspect for future versions

* chore: rename name of file icon

* chore: delete unneeded IAP icon

* chore: delete IAP part

* chore: delete IAP part

* chore: fix after merge

* chore: started to add courseRawImage

* chore: delete IAP part

* fix: after merge

* chore: deleted unsupported ios 15 modifiers

* chore: added courseRawImage

* chore: moved progress to correct place

* chore: fix broken tests

---------

Co-authored-by: Vadim Kuznetsov <[email protected]>
Co-authored-by: Anton Yarmolenko <[email protected]>
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.

3 participants