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

[Horizon] Module Sequence View #3076

Open
wants to merge 6 commits into
base: feature/horizon
Choose a base branch
from

Conversation

Ahmed-Naguib93
Copy link
Contributor

@Ahmed-Naguib93 Ahmed-Naguib93 self-assigned this Jan 22, 2025
@Ahmed-Naguib93 Ahmed-Naguib93 changed the title Feature/horizon clx 342 module sequence view [Horizon] Module Sequence View Jan 22, 2025
@inst-danger
Copy link
Contributor

inst-danger commented Jan 22, 2025

Fails
🚫 Build failed, skipping coverage check
❌ XCTest failed: CoreTests/AssignmentCellViewModelTests/testSubmissionStatusAndIconAndColor
XCTAssertEqual failed: ("#6a7883") is not equal to ("#697783")
XCTAssertEqual failed: ("#03893d") is not equal to ("#03893c")
❌ XCTest failed: CoreTests/CoreSearchHostingControllerTests/test_search_experience_started
XCTAssertEqual failed: ("") is not equal to ("Example")
❌ XCTest failed: CoreTests/DiscussionDetailsViewControllerTests/testAutomaticRead
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x112827600; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x600000720520>>".
XCTAssertNotNil failed
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x112827600; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x600000720520>>".
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x112827600; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x600000720520>>".
❌ XCTest failed: CoreTests/DiscussionDetailsViewControllerTests/testLayout
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x10808a400; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x60000070bf20>>".
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x10808a400; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x60000070bf20>>".
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x10808a400; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x60000070bf20>>".
XCTAssertTrue failed
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x10808a400; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x60000070bf20>>".
XCTAssertTrue failed
❌ XCTest failed: CoreTests/DiscussionDetailsViewControllerTests/testShowEntry
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x106052000; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x600000728fe0>>".
XCTAssertEqual failed: ("nil") is not equal to ("Optional("4")")
❌ XCTest failed: CoreTests/DiscussionDetailsViewControllerTests/testShowReplies
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x106055800; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x600000729860>>".
XCTAssertTrue failed
XCTAssertTrue failed
❌ XCTest failed: CoreTests/DiscussionDetailsViewControllerTests/testStudentGroupTopic
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x106059000; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x60000072a0e0>>".
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
❌ XCTest failed: CoreTests/DiscussionDetailsViewControllerTests/testStudentGroupTopicWhenUserNotInAGroup
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x11205de00; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x600000712980>>".
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed
❌ XCTest failed: CoreTests/DiscussionDetailsViewControllerTests/testTeacherGroupTopic
Asynchronous wait failed: Exceeded timeout of 9 seconds, with unfulfilled expectations: "Expect predicate `runningCount == 0` for object <_TtCC9CoreTests36DiscussionDetailsViewControllerTests11MockWebView: 0x112032800; frame = (0 0; 375 100); opaque = NO; backgroundColor = <UIDynamicCatalogColor: 0x600002381f90; name = backgroundLightest>; layer = <CALayer: 0x600000713200>>".
XCTAssertTrue failed
XCTAssertTrue failed
XCTAssertTrue failed

Generated by 🚫 dangerJS against a7c7742

@inst-danger
Copy link
Contributor

inst-danger commented Jan 22, 2025

Horizon Build QR Code:

@reabbotted
Copy link

I worry about how deeply rooted the AppEnvironment is passed through the classes. That makes all of those classes dependent on that AppEnvironment class. It would be best to limit those classes to only require just what they need to get their job done to make them more reusable.

@reabbotted
Copy link

It works well. My general comments are:

  • The AppEnvironment comment I made above
  • Using the HorizonUI components when possible
  • Avoid putting logic (if statements, calculations, assigning values to properties) inside of views as much as possible and move them to the view models.

Copy link
Contributor

@szabinst szabinst left a comment

Choose a reason for hiding this comment

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

Thank you for tackling this, this has been a pain point for a long time! Generally, it works well and the logic is well abstracted too.

I'd like to see some file reorganization inside the ModuleItemSequence folder.

  • The assembly file could live in the root like how other features are set up
  • The ModuleNavBar folder should directly contain those two files, and the ModuleNavBar folder itself could go under ModuleItemSequence/View/ModuleNavBar
  • Same is true for RepresentableViews folder

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.

4 participants