Problem/Motivation

Currently the Courses listing is a simple unformatted list of links. It would have a more polished look, and be more informative, if they are displayed as cards.

Each course card could include:

  • title
  • image
  • description summary
  • prerequisites
  • status
  • progress indicator
  • tags
  • action buttons

Some of these will need extra fields to be added to the lms_course group type, which may involve an upgrade path.

Issue fork lms-3518648

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

ob3ron created an issue. See original summary.

graber’s picture

graber commented

I think that’s site building and theming.. not sure if this should be in LMS core, every project will customize that anyway.

graber’s picture

graber commented

Maybe providing some default components is a good idea though. The question is what’s the best place for them?

ob3ron’s picture

ob3ron commented

I was thinking just to set up a basic SDC card component, possibly with a default generic course image. Then site builders can customize in their theme or through UI Suite. It can live in a course_card directory under /components.

catch’s picture

catch
he/him
Primary language English
commented

The extra fields are the sort of thing that would best live in an LMS recipe / site template along the lines of Drupal CMS. I'm not sure what this means for an SDC card component in terms of where it could live or what it could support. If we only included title + a media image field that might be doable, although does UI suite or similar provide one like that already?

ob3ron opened merge request !99

ob3ron’s picture

ob3ron commented
Assigned: Unassigned » ob3ron
Status: Active » Needs work

ob3ron changed the visibility of the branch 3518648-display-course-listings to hidden.

ob3ron changed the visibility of the branch lms-3518648 to hidden.

ob3ron opened merge request !104

ob3ron’s picture

ob3ron commented
Status: Needs work » Needs review

Added a course card SDC in a responsive grid view, plus a views plugin that allows site builders to optionally display fields of their choice in the SDC that they've added to their Course group. Includes options for assigning fields to course card background, description, and two custom fields.

Notes:

  • May depend on #3515744: Convert course navigation block to SDC to be merged first to avoid conflicts.
  • Includes an update hook that overwrites the existing /courses view, or adds it if not present.
  • It would be good to coordinate release with a documentation update explaining the view options.
graber’s picture

graber commented
Status: Needs review » Needs work

1. Latest changes from 1.0.x need to be included
2. Some coding standards tests fail
3. We should avoid having a hard dependency on media module.

ob3ron’s picture

ob3ron commented
Status: Needs work » Needs review

Latest changes from 1.0.x included, and hard dependency on media module removed.

The start_link component needed to be refactored to work in all contexts.

ob3ron’s picture

ob3ron commented

To clarify the architectural changes:

StartLinkFieldItemList was both building and rendering the start_link component. That led to context-dependent fatal errors when a form was rebuilt via an AJAX call (like adding a lesson to a new course), and when displaying a start_link component from within a views plugin like our new CourseCardFormatter.

So the building and rendering responsibilities were separated:

StartLinkFieldItemList now only calculates the component props and stores them as a serialized string. It gets its context directly from the entity it is attached to.

A new field formatter LmsComponentFormatter calls the field's view() method, which takes the serialized props, unserializes them, and builds the component render array.

CourseStartLink no longer needs the getRenderable() method. Attributes were added for styling of each of the start_link statuses.

ob3ron’s picture

ob3ron commented

Documentation for configuring the course cards added at https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib...

graber’s picture

graber commented
Status: Needs review » Needs work
ob3ron’s picture

ob3ron commented
Status: Needs work » Needs review

Bouncing back for review after incorporating all feedback.

graber’s picture

graber commented
Status: Needs review » Needs work

Left some more comments on the MR, let's strip this as much as possible from extra fields and config in favor of exposing API if needed. Looks great otherwise.

ob3ron’s picture

ob3ron commented
Status: Needs work » Needs review

Addressed all review comments. Reinstated hook_update_N to set the field display format of the start_link component, which is necessary for all sites, but it does not install the updated /courses view.

  • graber committed 301896ef on 1.0.x authored by ob3ron 
    Issue #3518648: Display course listings as card components
graber’s picture

graber commented
Assigned: ob3ron » Unassigned
Status: Needs review » Fixed

Merged this after making some changes:
Dropped custom fields UI and renamed to `extra_fields`. Those can now be added by the new hook
Dropped the second module handler invoke as that can be done in a theme preprocess function
Constructor property promotion in the new Views row plugin
Removed duplicated custom fields code from component twig

Please create follow-ups if needed, I think we'll need a bit of automated test coverage for that, in short visit the view page after some course tests are done and check if start link text is as expected and if other fields contain correct values in a foreach loop.

catch’s picture

catch
he/him
Primary language English
commented

Hadn't reviewed this yet. The default view is added in config/install, but LMS doesn't have a views dependency, so I think that should be in config/optional?

graber’s picture

graber commented

Ahh, correct, I'll move it. Somehow thought we have that dependency but it's installed only in automated tests and obviously in the kickstarter repo.

graber’s picture

graber commented

Ok. There is a views dependency in the .info file actually.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.