Skip to content

Assessment: New Edit Assessment form#5034

Merged
ekowidianto merged 37 commits intomasterfrom
phillmont/new-edit-assessment
Sep 6, 2022
Merged

Assessment: New Edit Assessment form#5034
ekowidianto merged 37 commits intomasterfrom
phillmont/new-edit-assessment

Conversation

@pchunky
Copy link
Copy Markdown
Contributor

@pchunky pchunky commented Aug 30, 2022

Before After
image image

Notable changes

  • Fields are now categorised and responsive
  • Unavailable fields are now disabled and clearly annotated
  • Radio buttons for Visibility and Grading mode
  • Submission password is now Session unlock password, available when Session protection is enabled
  • New FileManager with multi-file selection support
  • Replace toggle switches with checkboxes for better discoverability
  • In the New Assessment dialog, the prompt to discard changes will only appear when the form has been modified
  • In the New Assessment dialog, Submit has been relabeled Create as Draft
  • AssessmentForm/index is now in TypeScript
  • Add AssessmentForm and AssessmentIndex tests in TypeScript with React Testing Library

Notes on testing inputs on FormTextFields

It is made known that our FormTextField component's value currently cannot be programmatically changed due to the way the onChange listener is written. Therefore, tests involving entering texts into FormTextFields will not work either because as per Testing Library's documentation, this is how one does it:

const input = form.getByLabelText('Title *');
fireEvent.change(input, { target: { value: 'New Title' } });
expect(input).toHaveValue('New Title');     // <- will fail!

which involves an dispatchEvent of type 'change' to the HTMLElement.

Until or unless FormTextField.onChange is changed, developers should use @testing-library/user-event to simulate "a more natural typing behaviour" and test FormTextField inputs. See 2afe473. For example:

import userEvent from '@testing-library/user-event';

const user = userEvent.setup();
const input = form.getByLabelText('Title *');   // <- assume value is currently 'Hello'
await user.type(input, ' World');
expect(input).toHaveValue('Hello World');       // <- will succeed!

user-event can also test for clicks and other user interactions, however, the author suggests using it only when the built-in fireEvent fails.

@pchunky pchunky requested a review from ekowidianto August 30, 2022 02:47
@pchunky pchunky self-assigned this Aug 30, 2022
@pchunky pchunky force-pushed the phillmont/new-edit-assessment branch 3 times, most recently from 624d09d to 40d0c6d Compare August 30, 2022 07:27
Copy link
Copy Markdown
Member

@ekowidianto ekowidianto left a comment

Choose a reason for hiding this comment

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

Hi @purfectliterature awesome work!! Thanks for this!

Some comments and nits below:

  • the save button placement is off in small screen.
    image
  • Remove paddings/reduce row height in the material table
    image
  • Can add download link for the materials uploaded (open in new tab)

Comment thread client/app/bundles/course/achievement/components/forms/AchievementForm.tsx Outdated
Comment thread client/app/lib/components/layouts/Section.tsx Outdated
Comment thread client/app/lib/components/layouts/Section.tsx
Comment thread client/app/lib/components/layouts/Section.tsx Outdated
Comment thread client/app/lib/components/course/ConditionList/index.jsx Outdated
@pchunky pchunky force-pushed the phillmont/new-edit-assessment branch 3 times, most recently from 940a847 to 01181ee Compare September 2, 2022 09:25
@pchunky
Copy link
Copy Markdown
Contributor Author

pchunky commented Sep 2, 2022

Request Current state Remarks
Fix Save button placement on mobile screens image Styling for this button is temporary. The New Assessment button is injected to .pull-right from Rails, but I did not want to do it that way. Once the entire header is ported to React, then we can properly place this.
Reduce row height in FileManager image None
Add URL to view files in new tab image Only uploaded materials will have this hyperlink.

@pchunky pchunky requested a review from ekowidianto September 2, 2022 10:10
@pchunky pchunky force-pushed the phillmont/new-edit-assessment branch 3 times, most recently from b5cf6f1 to 712f6fe Compare September 5, 2022 03:22
pchunky and others added 25 commits September 6, 2022 09:07
* MaterialUploader is now deprecated
* FileManager supports multiple files deletion
* Messages are now emitted using react-toastify
* FileManager test is written using react-testing-library
@ekowidianto ekowidianto force-pushed the phillmont/new-edit-assessment branch from 712f6fe to 4198e59 Compare September 6, 2022 03:12
Copy link
Copy Markdown
Member

@ekowidianto ekowidianto left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

@ekowidianto ekowidianto merged commit bc11fda into master Sep 6, 2022
@ekowidianto ekowidianto deleted the phillmont/new-edit-assessment branch September 6, 2022 03:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants