Moodle

have multiple renderers belonging to one plugin

Details

  • Type: Sub-task Sub-task
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 2.0
  • Fix Version/s: 2.0
  • Component/s: Libraries
  • Labels:
    None
  • Affected Branches:
    MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_20_STABLE

Description

It appears to be useful to allow an extra word in the plugin renderer class name, like moodle_mod_workshop_subthing_renderer - so you can have multiple renderers belonging to one plugin.

See http://moodle.org/mod/cvsadmin/view.php?conversationid=2705 for the background

Activity

Hide
David Mudrak added a comment -

Fixed URL

Show
David Mudrak added a comment - Fixed URL
Hide
David Mudrak added a comment -

See the attached 0001 patch for the proposed implementation

If you are using git, you can run
git am <<the patch filename>>
or - if it fails - try
git am -3 <<the patch filename>>

Non-gitters will just remove some lines from the top of the patch file to get pure diff

Show
David Mudrak added a comment - See the attached 0001 patch for the proposed implementation If you are using git, you can run git am <<the patch filename>> or - if it fails - try git am -3 <<the patch filename>> Non-gitters will just remove some lines from the top of the patch file to get pure diff
Hide
Tim Hunt added a comment -

That looks good.

I assume you have run the unit tests, and they all pass. Please comment. Thanks.

Show
Tim Hunt added a comment - That looks good. I assume you have run the unit tests, and they all pass. Please comment. Thanks.
Hide
David Mudrak added a comment -

Yes, the tests passed. I modified some of the current tests a bit - I know I should not do that but I felt I am doing right thing, hopefully. Also, I added some more to test the variations of calls with the new parameter. The new parameter works for my needs.
To be reviewed more carefully: how I integrated this into the templateable renderers as I am not very confident in this area. Thanks in advance.

Show
David Mudrak added a comment - Yes, the tests passed. I modified some of the current tests a bit - I know I should not do that but I felt I am doing right thing, hopefully. Also, I added some more to test the variations of calls with the new parameter. The new parameter works for my needs. To be reviewed more carefully: how I integrated this into the templateable renderers as I am not very confident in this area. Thanks in advance.
Hide
Tim Hunt added a comment -

Doh! that was an interesting typo. I mean to say Please commit.

However, you are right template_renderer_factory needs a bit more work. In get_renderer, the $searchpaths array needs to

$path = $rootpath . '/' . $module;
if (!is_null($subtype)) { $path .= '/' . $subtype; }
if (is_dir($path)) { $searchpaths[] = $path; }

Is probably a good way to do it.

Anyway, template renderer is only an experimental proof-of-concept. No need to be too concerned with it now.

Show
Tim Hunt added a comment - Doh! that was an interesting typo. I mean to say Please commit. However, you are right template_renderer_factory needs a bit more work. In get_renderer, the $searchpaths array needs to $path = $rootpath . '/' . $module; if (!is_null($subtype)) { $path .= '/' . $subtype; } if (is_dir($path)) { $searchpaths[] = $path; } Is probably a good way to do it. Anyway, template renderer is only an experimental proof-of-concept. No need to be too concerned with it now.
Hide
David Mudrak added a comment -

Well, this was my original intention to keep all templates in one directory. But you are right, subdirectories templates should respect the subtypes.

p.s. I like the typo. Maybe we can start using "Fix a lot" instead "Thanks a lot" and so on

Show
David Mudrak added a comment - Well, this was my original intention to keep all templates in one directory. But you are right, subdirectories templates should respect the subtypes. p.s. I like the typo. Maybe we can start using "Fix a lot" instead "Thanks a lot" and so on
Hide
David Mudrak added a comment -

Committed. I have fixed the template_renderer_factory::get_renderer() so it sets up folders for subtypes. This involved a little extension of the unit test.

Show
David Mudrak added a comment - Committed. I have fixed the template_renderer_factory::get_renderer() so it sets up folders for subtypes. This involved a little extension of the unit test.
Hide
Tim Hunt added a comment -

Great. Thanks.

Show
Tim Hunt added a comment - Great. Thanks.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: