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

feat: allow to view in CMS #25

Merged
merged 5 commits into from
Dec 8, 2023
Merged

Conversation

Ian2012
Copy link
Contributor

@Ian2012 Ian2012 commented Sep 26, 2023

Description

This PR allows to preview of the h5p content in the CMS. This will avoid the need for instructors to publish the unit to reproduce the h5p content.

This change is part of a series of improvements to the h5pxblock.

Before

image

After

image

@Ian2012 Ian2012 marked this pull request as ready for review September 28, 2023 19:14
Copy link
Contributor

@mariajgrimaldi mariajgrimaldi left a comment

Choose a reason for hiding this comment

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

Can we add a before and after in the cover letter? Also, can we include why are we proposing this change?

@Ian2012
Copy link
Contributor Author

Ian2012 commented Oct 24, 2023

@ziafazal can you review this PR?

@mariajgrimaldi
Copy link
Contributor

Hi @ziafazal! Friendly ping on this :)

@ziafazal
Copy link
Contributor

ziafazal commented Dec 4, 2023

Hi @ziafazal! Friendly ping on this :)

Hi @mariajgrimaldi & @Ian2012

Sorry for the delay. I'll review it this week.

Copy link
Contributor

@ziafazal ziafazal left a comment

Choose a reason for hiding this comment

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

@Ian2012 overall looks good. Just one thing needs to be addressed?

@@ -200,16 +199,12 @@ def get_context_studio(self):
"h5p_xblock": self,
}

def author_view(self, context=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are removing author_view we should remove author_view.html and also relevant translations in *.po files of all supported languages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ziafazal ziafazal merged commit 6dec211 into edly-io:master Dec 8, 2023
1 check passed
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.

5 participants