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

Edit modal - add additional fields to "Basic info" section #3370

Merged
merged 18 commits into from
Apr 28, 2022

Conversation

sairina
Copy link
Contributor

@sairina sairina commented Apr 15, 2022

Summary

Description of the change(s) you made

  1. Added the following fields to the "Basic Information" section of the edit modal:
  • "learning activity"
  • "level"
  • "what you will need"
  1. Updated "Basic Information" section to be a 2-column format
  2. Moved "Tags" to the 2-column format

Note, this PR does not cover the "Categories" field, and the following issues are to address some specifics with regards to the "Learning Activity" dropdown field and setting defaults based on content kind, which is not covered in this PR.

Manual verification steps performed

In the edit modal:

  1. Added content into every field (description, learning activity, level, what you will need, tags)
  2. Clicked Finish
  3. Went back into the resource and checked that the content was still there
  4. Removed all learning activities (empty field)
  5. Checked to see that validation is working for that field (learning activity is required)

Screenshots

Validation working for "Learning activity"
Screen Shot 2022-04-20 at 10 42 03 AM
2-column format
Screen Shot 2022-04-20 at 10 42 47 AM
As inputs in right column change in height, so does "Description"
Screen Shot 2022-04-20 at 10 43 09 AM
Mobile view/1-column
2022-04-20 10 44 55

Does this introduce any tech-debt items?

This is a continuation of this issue #3205


Reviewer guidance

How can a reviewer test these changes? Please see Gherkin scenarios for additional, specific user interactions

In the edit modal:

  1. Added content into every field (description, learning activity, level, what you will need, tags)
  2. Clicked Finish
  3. Went back into the resource and checked that the content was still there
  4. Removed all learning activities (empty field)
  5. Checked to see that validation is working for that field (learning activity is required)
  6. Check that the tests are accurately reflecting changes made in code

References

Comments


Contributor's Checklist

Studio-specifc:

  • All user-facing strings are translated properly
  • The notranslate class been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)
  • All UI components are LTR and RTL compliant
  • Views are organized into pages, components, and layouts directories as described in the docs

Testing:

  • Code is clean and well-commented
  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Any new interactions have been added to the QA Sheet
  • Critical and brittle code paths are covered by unit tests

Reviewer's Checklist

This section is for reviewers to fill out.

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

A couple of things that could be tweaked, but nothing alarming. I have not yet manually tested.

@sairina sairina force-pushed the metadata-basic-info branch from 13b58ab to d6a0d94 Compare April 26, 2022 00:40
return Object.entries(ContentLevels).map(level => {
let translationKey;
if (level[0] === 'PROFESSIONAL') {
translationKey = 'specializedProfessionalTraining';
Copy link
Member

Choose a reason for hiding this comment

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

cc @marcellamaki we might want to consolidate this logic somewhere like I think we did on Kolibri?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Test could still do more guarantee of behaviour.

@sairina sairina requested a review from rtibbles April 27, 2022 22:41
@rtibbles rtibbles merged commit b7eba64 into learningequality:unstable Apr 28, 2022
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.

2 participants