RunestoneInteractive / RunestoneComponents

Packaging of the Runestone tools for publishing educational materials using github pages
http://runestoneinteractive.org
Other
101 stars 225 forks source link

DRAFT: initial commit of quizly component #1168

Closed ram8647 closed 3 years ago

ram8647 commented 3 years ago

Making a draft pull request

bnmnetp commented 3 years ago

The test folder is missing the pavement.py and conf.py files. If you could add them and push that would be great and I'll be able to build the test pages.

ram8647 commented 3 years ago

Can you pull my khanex changes from this PR?

bnmnetp commented 3 years ago

This gets it into the database! But, We have to figure out what to do about the src= path for the iframe. src=../_static does not work.

The problem is that we are injecting this html (from the database) into a page where the ../_static does not even exist. This is progress, but also the first iframe based component we have tried to fully incorporate into the whole grading system.

We need to have nginx see the src url as /runestone/books/published/mobilecsp/_static/.... when we save it to the database. There is an easy hack it solution, and the right solution... I'm still thinking, but we may start with the easy hack of just rewriting the url when we write to the database.

bjones1 commented 3 years ago

@ram8647, my name is Bryan Jones and I'm a core developer for Runestone. @bnmnetp asked me to provide some feedback and help on your PR. Here are some quick thoughts; feel free to ask questions and I'll be happy to help!

I just added a feature that loads only the JavaScript needed for each page. For example, if a page doesn't have an ActiveCode exercise, then the ActiveCode JS (>1 MB) doesn't get loaded. I'd like to apply this same approach to your components -- add the required JS for them only when needed. This is fairly simple, and I'll be happy to walk you through it.

Add your new JS as a dynamic import. Here's the static import you included in the current PR's webpack.config.js:

module.exports = (env) => {
    return {
        entry: [
            "./runestone/quizly/js/quizly.js",
            "./runestone/khanex/js/khanex.js",

Here's the dynamic equivalent -- this goes in webpack.index.js in the current repo:

// Dynamically loaded components
// =============================
// This provides a list of modules that components can dynamically import. 
// Webpack will create a list of imports for each based on its analysis.
const module_map = {
    // Wrap each import in a function, so that it won't occur until the function 
    // is called. While something cleaner would be nice, webpack can't analyze 
    // things like ``import(expression)``.
    //
    // The keys must match the value of each component's ``data-component`` 
    // attribute -- the ``runestone_import`` and ``runestone_auto_import`` functions 
    // assume this.
    khanex: () => import("./runestone/khanex/js/khanex.js"),
    quizly: () => import("./runestone/quizly/js/quizly.js"),
bnmnetp commented 3 years ago

@bjones1 some version of what you just wrote for Ralph would be great to add do the documentation for the components so that new component authors understand what they should do.

bjones1 commented 3 years ago

Good point. Currently, the docs are at https://github.com/RunestoneInteractive/RunestoneComponents/blob/master/webpack.index.js. I'll send a PR which includes a link to this from CONTRIBUTING.rst. If you can think of other relevant places for docs/links, let me know.

bjones1 commented 3 years ago

Done in #1209.

ram8647 commented 3 years ago

That's a good point. I can look into storing them in the MobileCSP static folder. What do you think? -- ralph

On Sun, Jun 20, 2021 at 7:57 PM Bradley Miller @.***> wrote:

@.**** commented on this pull request.

In runestone/khanex/js/exercises/absolute_value.html https://github.com/RunestoneInteractive/RunestoneComponents/pull/1168#discussion_r655007261 :

@@ -0,0 +1,53 @@ +<!DOCTYPE html>

Are all of these exercises needed as part of the RunestoneComponents? It seems like this would be better to have as part of a book...

Also, I can't find anywhere in the code that these files are referenced.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/RunestoneInteractive/RunestoneComponents/pull/1168#pullrequestreview-687953194, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAQGKB6B5OG7OAHZI7W7TR3TTZ56PANCNFSM42RZWVSQ .

bnmnetp commented 3 years ago

The components should be generic in the sense that they can be used to add questions to any book. In other words we should not have to make a new release of the runestone components in order to add a new question to a book. So, yes, I think these should be in mobileCSP if at all possible.