mudrd8mz / moodle-tool_pluginskel

Generator of Moodle plugins skeletons
https://moodle.org/plugins/tool_pluginskel
Other
51 stars 46 forks source link

Initial version of the web interface #59

Closed alexandru-elisei closed 8 years ago

alexandru-elisei commented 8 years ago

This is just a prototype, I know it looks ugly at places, this is just the initial version to show you how we can generate the web page automatically based on the templates needed to create a plugin. If you agree with it I'll modify it to make it look better.

I've tested the web interface, right now I can:

I've created a plugin from a saved recipe, so that works fine.

There's also some fixes to other files.

While you revise this commit I'll work on creating the get_template_variables() function for each skel type, no matter what we decide about the web interface we're still going to need a function like that.

alexandru-elisei commented 8 years ago

I've rebased against master.

I've also modified an error that I introduced when modifying the way step1_form is generated, I think you will need to clone the branch with these changes for the web interface to work properly.

mudrd8mz commented 8 years ago

Hi Alex. Sorry I did not have a chance to look at you recent changes. I am leaving tomorrow and will be out of office for the next week. I will check the work in progress once I am back on 8 August.

alexandru-elisei commented 8 years ago

Sure, no problem, after all it's still a work in progress.

About the travis failed check, I'm getting a DOM node value and transforming it into a number like this:

var count = +$('#id').val();

Travis throws an error because I have no space between the '+' and the '$'. This is the coding style that I saw recommended on the jquery website.

Should I change how I wrote that?

mudrd8mz commented 8 years ago

I haven't seen this syntax used before, what is it good for?

alexandru-elisei commented 8 years ago

The string returned by .val() is automatically converted into a number. I need to increment <variable>count after adding new fields to the recipe.

alexandru-elisei commented 8 years ago

This is a final version.

I have implemented all the features except adding the index in front of array variables. I will work on it while you take a look at the code.

There is a lot of code and I've tried to make it as simple and easy to follow as possible, but I'm sure there'll be some things that I'll need to rewrite.

There is one limitation on the recipe variables: there cannot be more than two levels of array nesting. For example:

capabilities:
    - name: cap
      ...
    - archetypes:
        - role: student
        - permission: CAP_ALLOW

It has two levels of nesting (archetypes represents the second level of nesting).

So is backup_moodle2:

mod_features:
    backup_moodle2:
        restore_task: true
        ...
        backup_elements:
            - name: first
            - name: second

In this case, backup_moodle2 is an associative array which has a numerically indexed array (backup_elements) as one of the values.

Implementing more than two levels of nesting seems useless at this time (we don't have a recipe configuration that requires that).

Also, I haven't implemented two levels of nesting for associative arrays. We don't have a recipe option that looks like that, and we can express two associative arrays nested inside each other as one big associative array if we need to.

mudrd8mz commented 8 years ago

Thanks Alex for this. I generally like the implementation and I am going to merge it now. Beside the comments left in the code (see above), there are two major things that I think should be addressed yet:

Code-wise, the plugin is cool. We just need to improve the UI a bit (in terms of intuitiveness and built-in help) as that is the first impression that most people will have.

alexandru-elisei commented 8 years ago

Because some fields are not unique (for example, I am using name for a lot of variable options), an unique name for the help icons is needed. I can use their position in the recipe to create an unique name for the help string.

For example:

mobile_addons:
    - name: 'my_first_addon'
      dependencies:
          - name: dependency_one

The help string for the my_first_addon name would be mobile_addons_name_help, and for the first depenedency name would be mobile_addons_dependencies_name_help. These would be passed to the add_<mform elemens>() functions as an argument.

mudrd8mz commented 8 years ago

Because some fields are not unique (for example, I am using name for a lot of variable options)

Let me discourage from that. We learnt that it is better to have explicit own string for every field even if in English they read the same. For example in Czech, we have different translation of "Name" depending on whether it is name of a thing or a name of a person.

So I would suggest to have explicit strings like mobile_addons_name and mobile_addons_dependencies_name even if they both are defined as "Name" in the English pack.

At the end, you are likely to realise that having them all defined more explicitly like "Capability name", "Addon name", "Dependency name" etc. is actually better than simple "Name" everywhere.