Closed IvanStepanok closed 3 weeks ago
Thanks for the pull request, @IvanStepanok! Please note that it may take us up to several weeks or months to complete a review and merge your PR.
Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.
Please let us know once your PR is ready for our review and all tests are green.
The arrow is pointing in the wrong direction
By design, arrows should be displayed only in sections with assignments. | Design | Device |
---|---|---|
It's not really critical, but the animation works weird when there is only 1 element in it. The element appears before the container is resized.
https://github.com/openedx/openedx-app-ios/assets/127732735/93291019-d71b-459d-a2b0-03bf634a1de4
@volodymyr-chekyrta Thanks for the review! All requested changes have been made🙌
Hi @IvanStepanok, this looks great overall! A couple of design questions/feedback I wanted to check on:
shade
color #F9FAFB
as their background color. It looks like this might be missing from the videodark theme accent text
color #879FF5
for contrast/accessibility reasonsBoth of these are color-related, and the video compression makes it a bit ambiguous whether these colors are already being used, so I wanted to confirm
Hi @sdaitzman and thanks for paying so much attention to details! Please, take a look at my changes. The download icon had the wrong color, but in the light theme, not in the dark☺️
Thanks so much @IvanStepanok! Just to confirm, is the download icon in dark mode using #879FF5
? In that screenshot, I see a value of #5378F8, but it's possible that's just an image encoding/color space variation.
Other than that, everything looks great to me/ready to merge from a design standpoint, thanks for these updates!
Hi, @sdaitzman For some reason when you load icons and make a screenshot, they have a little difference in color measurement. I think is because of different color profiles used on real device and the simulator. I found a topic about this, but anyway, I think all we can do is just accept the rules🙌
@IvanStepanok 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.
Quick Preview
https://github.com/openedx/openedx-app-ios/assets/128456094/dd78b250-b349-4b58-9f11-7b820e50e0fe
Pay attention ⚠️
❌ "COURSE_NESTED_LIST_ENABLED" removed ✅ "COURSE_DROPDOWN_NAVIGATION_ENABLED" added
Old implementations in CourseStructureNestedListView.swift and CourseStructureView.swift are removed.
Design: https://www.figma.com/design/iZ56YMjbRMShCCDxqrqRrR/Open-edX-Mobile-App-All-Screens-v2.1?node-id=9177-44208&t=kHcQmxzck33FVrAT-0