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

Implementation for Edit Assessment dialogue (1/2) #252

Closed
cloudyyoung opened this issue Mar 20, 2023 · 5 comments · Fixed by #275
Closed

Implementation for Edit Assessment dialogue (1/2) #252

cloudyyoung opened this issue Mar 20, 2023 · 5 comments · Fixed by #275
Assignees

Comments

@cloudyyoung
Copy link
Member

Background

The current desktop user interface is problematic, which doesn't provide a valid user experience to perform primary actions and accomplish user tasks:

  1. The assessment list panel and pdf panel are not incorporating
  2. Edit assessment dialogue implementation is different from what was defined in the prototype

These two problems must be resolved before we can publish this project as a complete product.

Editing assessment dialogue specifications

Desktop

The Dialogue should be a fixed floating panel above assessment list and must not scroll with the window scrollbar. User should be able to scroll the Dialogue independently from PDF viewer. Regardless of which assessment card user has opened, the position of the window scroll bar must not change when an assessment is clicked for editing, because user should not be jumping around within the assessment list.

Note: This could be complicated to implement, but it only makes sense when the Dialogue is a floating panel. I have also suggested other alternative approaches previously, we should switch to one of them if this is not feasible.

Alternative 1: Regular dialogue

The simplest alternative is to open a regular big dialogue which has editing panel on the left and pdf on the right again.

Alternative 2: Expanding card

Another easier alternative is that assessment card is expanded into editing panel. Only one card will show its editing panel at a time.

Mobile

The Dialogue is a full-screen dialogue. It technically is almost an independent page but with presence transition animation from the bottom. The dialogue should be floating above the course page and must not live within course page as a child panel. The Dialogue has an AppTopBar component (small version) allowing user to save or close the dialogue. The Dialogue must fill the entire screen.

See: https://m3.material.io/components/dialogs/guidelines#63783a40-cb25-46d4-9cd6-6f6ed43e4650

Please leave comments if you have any questions/concerns, eg. how the expected behaviour should be looking like.

@tim-macphail
Copy link
Collaborator

The current desktop user interface is problematic, which doesn't provide a valid user experience to perform primary actions and accomplish user tasks:

It allows the user to perform all the primary actions it was intended to do.

  1. The assessment list panel and pdf panel are not incorporating

Is this referring to the scrolling behaviour?

  1. Edit assessment dialogue implementation is different from what was defined in the prototype

It was implemented as it was defined in the prototype. The prototype since changed. Please be aware of how comments like these come across.

Lastly, let's do Alternative 2: Expanding card

@cloudyyoung
Copy link
Member Author

cloudyyoung commented Mar 22, 2023

For the first two points, I will reply in the comment section in #253.


It was implemented as it was defined in the prototype. The prototype since changed. Please be aware of how comments like these come across.

For desktop Edit Assessment Panel, though several different approaches have been discussed on how the panel should present itself, the fact that the Panel should be a floating element has never been changed since the very first prototype.

Currently, Edit Panel lives in CoursePanel.tsx and its display is determined by the state variable editingAssessment. When the Panel is displaying, the AssessmentList is removed from the DOM and will be rendered again later on, which causes problem no. 2 as mentioned above.

For mobile Edit Assessment Panel, the prototype in fact has never been changed since the very first version either. In both versions of prototype, the Panel is transitioned from the bottom and takes the entire screen:

Screen.Recording.2023-03-22.at.1.44.12.PM.mov

In our current implementation, the Panel is not taking up the entire screen. The video below provides a comparison:

RPReplay_Final1679514437.1.mp4

From both mobile and desktop prototypes, I believed it should be fairly straight-forward for developers to come to the conclusion that the Panel is somewhat an independent page or a float dialogue. Indeed, there are similar concepts that would be applied here in Material Design 3 where a full-screen dialogue shows on mobile if the dialogue contains a form (please see the link above) - which is what the mobile version prototype exactly presents.

Though #234 improves, it is not addressing the Dialogue issue described in here.

This Issue is not to disqualify any work for Edit Assessment Panel. In the opposite, I realize that there is unclarity for the expected behaviour for the Panel, which leads to an unexpected implementation that causes mentioned problem previously. This Issue is to:

  1. Give a fully detailed description and expected behaviour for the Panel as I can - which was not mentioned in Develop Edit Assessment Dialogue #135, for documentation and implementation purpose
  2. Note down that there are usability issues with the existing implementation, the implementation should be modified to what was the prototype trying to show, in order to resolve usability issues

Up to this point, I shall also give a rough approach of how the Panel implementation should be changed:

  1. Possibly introduce Headless CSS and utilize its Dialogue component
  2. Remove the Panel from where it is now in CoursePanel.tsx
  3. Wrap the EditAssessment component in a Dialogue, show the dialogue whenever editingAssessment is true

Which is a fairly easy fix.

@tim-macphail
Copy link
Collaborator

Sounds good, I think in the early prototypes there was just the mobile dialogue, and I assumed it was more of a component that could be rendered into desktop or mobile screens rather than a whole screen just for mobile

@tim-macphail
Copy link
Collaborator

image
I think we should scrap the modality field and the plus button below it.

  1. Modality is implicit in location (assuming modality means [in person/online])
  2. There is an iCal event property for Location, not modality
  3. Anything you want can be put into the notes folder
  4. What happens when you press the plus button to add more fields? I've looked around and can't find it in the figma
  5. Modality and additional fields will all have to appear in the "description" property of an iCal event anyways. It seems simpler to write it in the Notes section to begin with. (Notes maps to iCal event "description")

@cloudyyoung
Copy link
Member Author

image I think we should scrap the modality field and the plus button below it.

  1. Modality is implicit in location (assuming modality means [in person/online])
  2. There is an iCal event property for Location, not modality
  3. Anything you want can be put into the notes folder
  4. What happens when you press the plus button to add more fields? I've looked around and can't find it in the figma
  5. Modality and additional fields will all have to appear in the "description" property of an iCal event anyways. It seems simpler to write it in the Notes section to begin with. (Notes maps to iCal event "description")

The prototype has been updated according to these points.

The idea of having different fields is to show variety of dimensions of an assessment to users (and by our algorithm). A list of pre-defined fields such as location, modality, people, etc can be extracted and display in the format of fields, and based on that users can add customized fields by pressing the add button to enrich the assessment info.

It seems like the algorithm has not been able to recognize that much information so the prototype stops designing in that direction, and now we are simply putting anything in the Notes section.

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 a pull request may close this issue.

2 participants