Skip to content

Conversation

ndrsszsz
Copy link
Contributor

@ndrsszsz ndrsszsz commented Oct 22, 2024

refs: MBL-14876
affects: Student, Teacher
release note: Added functionality to filter and group assignments.
test plan: See ticket

Assignment List screen

  • Add Filter icon in Navigation Bar (should turn to solid if there's an active filtering and that differs from the default settings).
  • Upgrade sections to be collapsable.

Assignment Filter screen

  • Grading Period section: Should only be visible if there are multiple Grading Periods (default option is All Grading Periods).
  • Group By section: Should always be visible (default option is Due Date).

Figma

Test Plan

See ticket.

Assignment Filter screen

  • Navigation bar title should contain the name of the course under the screen title.
  • Done button should always be visible and active.
  • Grading Period section of the screen should only be visible if there are multiple grading periods.
  • Default setting for grading periods is "All Grading Periods" right now. (Will be modified to the currently active grading period)
  • Sort By section should always be visible and should have "Due Date" option selected by default.
  • Changes are saved to userdefaults (except grading period settings).

Screenshots

Assignment List

BeforeAfter

Assignment Filter

BeforeAfter

Checklist

  • Follow-up e2e test ticket created
  • A11y checked
  • Tested on phone
  • Tested on tablet
  • Tested in dark mode
  • Tested in light mode
  • Approve from product

add - AssignmentFilterViewModel - enabling save button on change of previously set values
remove - AssignmentListView - old filter button
add - AssignmentFilterScreen - all button to enable selecting no grading periods
remove - unused code
update - completion func to use struct instead of tuple
update - unit tests
rename - AssignmentListView to AssignmentListScreen
affects: Student, Teacher
release note: Added Filter options to Assignment List
test plan: See ticket
add - userdefaults support
@ndrsszsz
Copy link
Contributor Author

Unit tests will be added later.

@inst-danger
Copy link
Contributor

inst-danger commented Oct 22, 2024

Warnings
⚠️ One or more files are below the minimum test coverage 50%

Release Note:

Added Filter options to Assignment List

Affected Apps: Student, Teacher

MBL-17932

Coverage New % Master % Delta
Canvas iOS 91.43% 91.71% -0.27%
Core/Core/Assignments/AssignmentList/ViewModel/AssignmentListViewModel.swift 47.8% 74.81% -27.01%

Generated by 🚫 dangerJS against 71c15a3

@inst-danger
Copy link
Contributor

inst-danger commented Oct 28, 2024

Teacher Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Oct 28, 2024

Student Build QR Code:

Copy link
Contributor

@rh12 rh12 left a comment

Choose a reason for hiding this comment

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

Great to see this coming together again after the design changes!

I've got a couple of small comments

@suhaibabsi-inst
Copy link
Contributor

QA + 1

@suhaibabsi-inst suhaibabsi-inst self-requested a review October 31, 2024 16:56
Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst left a comment

Choose a reason for hiding this comment

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

Code +1
QA + 1

Please address above comments at your convenience.

suhaibabsi-inst
suhaibabsi-inst previously approved these changes Nov 3, 2024
Copy link
Contributor

@suhaibabsi-inst suhaibabsi-inst left a comment

Choose a reason for hiding this comment

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

Thanks @ndrsszsz !

rh12
rh12 previously approved these changes Nov 4, 2024
Copy link
Contributor

@rh12 rh12 left a comment

Choose a reason for hiding this comment

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

QA + 1 again

@ndrsszsz ndrsszsz force-pushed the feature/MBL-17932-Assignment-list-improvement2 branch from 1042156 to fce281a Compare November 5, 2024 13:30
Copy link
Collaborator

@vargaat vargaat left a comment

Choose a reason for hiding this comment

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

I compared my assignment list from the app to web and our list misses a few assignments even when all filter options are turned on: 1 from "Middle Group" and 2 from "Test Assignment Group". I haven't checked "Test Assignment Group" since there are too many assignments there.

vargaat
vargaat previously approved these changes Nov 6, 2024
Copy link
Collaborator

@vargaat vargaat left a comment

Choose a reason for hiding this comment

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

The issue I discovered is not related to this PR.

QA+1

Copy link
Contributor

@rh12 rh12 left a comment

Choose a reason for hiding this comment

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

QA + 1 again

Found some minor VoiceOver issues around ChecboxCell subtitles and the Filter button, but these can wait for the next PR. Discussed details offline.

@ndrsszsz ndrsszsz merged commit cb2f937 into master Nov 6, 2024
5 of 6 checks passed
@ndrsszsz ndrsszsz deleted the feature/MBL-17932-Assignment-list-improvement2 branch November 6, 2024 14:28
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.

5 participants