Skip to content

Add course completion meta field#2877

Merged
outdoor2kode merged 6 commits intotrunkfrom
2564-add-course-completion-meta-field
Sep 10, 2024
Merged

Add course completion meta field#2877
outdoor2kode merged 6 commits intotrunkfrom
2564-add-course-completion-meta-field

Conversation

@outdoor2kode
Copy link
Copy Markdown
Contributor

@outdoor2kode outdoor2kode commented Sep 8, 2024

Resolves #2564

This PR adds two fields to the Gutenberg editor sidebar, allowing users to enter a custom success message and survey link.

Screenshots

image
image

Update

description added

image

@outdoor2kode outdoor2kode self-assigned this Sep 8, 2024
@outdoor2kode outdoor2kode added [Component] Learn Theme Website development issues related to the Learn theme. [Component] Learn Plugin Website development issues related to the Learn plugin. labels Sep 8, 2024
* Inserter: no
*/

$course_id = isset( $_GET['course_id'] ) ? intval( $_GET['course_id'] ) : get_the_ID();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious about this, did you find a case where get_the_ID() didn't work? I would have thought that would be the ideal means for getting it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I haven’t encountered this, but I remember being reminded before that in certain places like before/after the loop or in header, footer, or admin pages, get_the_ID() might not work, as it’s likely to return no value in those contexts. It works here because I saw this snippet from the post_template block.

const link = postMetaData?._course_completion_survey_link || '';

return (
<PluginDocumentSettingPanel title={ __( 'Course Completion Settings', 'wporg-learn' ) }>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Given this is displayed on the course edit screen in the settings panel I think we can simplify the text a bit, how about

Suggested change
<PluginDocumentSettingPanel title={ __( 'Course Completion Settings', 'wporg-learn' ) }>
<PluginDocumentSettingPanel title={ __( 'Completed screen', 'wporg-learn' ) }>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

<p>{ __( 'If the fields are left blank, the default values will be applied.', 'wporg-learn' ) }</p>
</PanelRow>
<PanelRow>
<TextControl
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we show a placeholder value here 'Congratulations on completing this course!'?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image
The placeholder is getting cut off though. What do you think about shortening it to something like: 'Course completed!' or just leaving it blank like the other fields?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Personally I don't have a problem with it being cut off; I think it still provides an example of what is expected. If we did want to shorten it I'd just use Congratulations!, as it's still close to the default.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it still provides an example of what is expected.

This makes sense to me. 16df720

return (
<PluginDocumentSettingPanel title={ __( 'Course Completion Settings', 'wporg-learn' ) }>
<PanelRow>
<p>{ __( 'If the fields are left blank, the default values will be applied.', 'wporg-learn' ) }</p>
Copy link
Copy Markdown
Contributor

@adamwoodnz adamwoodnz Sep 9, 2024

Choose a reason for hiding this comment

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

Suggested change
<p>{ __( 'If the fields are left blank, the default values will be applied.', 'wporg-learn' ) }</p>
<p>{ __( 'These fields customize what is displayed on the Course Completed screen. If left blank, the default values will be applied.', 'wporg-learn' ) }</p>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

Nice work! A few suggestions inline.

Copy link
Copy Markdown
Contributor

@adamwoodnz adamwoodnz left a comment

Choose a reason for hiding this comment

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

LGTM

}

/**
* Register post meta keys for lessons.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Register post meta keys for lessons.
* Register post meta keys for courses.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@outdoor2kode outdoor2kode merged commit bcce0c1 into trunk Sep 10, 2024
@outdoor2kode outdoor2kode deleted the 2564-add-course-completion-meta-field branch September 10, 2024 05:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Component] Learn Plugin Website development issues related to the Learn plugin. [Component] Learn Theme Website development issues related to the Learn theme.

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Course completion success message

2 participants