Block title links should be made optional

Bug #617159 reported by Richard Mansfield
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
Fix Released
Medium
Eugene

Bug Description

Blocks that contain a single artefact are automatically rendered with a link around the block title pointing to view/artefact.php?artefact=x&view=y

That is annoying for artefact plugin authors who haven't implemented a render_self function, because that page is broken without one.

There are two things we should do to solve this.

(1) Either
- make render_self abstract in the base class (force plugin authors to write it as it's used in html export anyway); or
- implement a very plain render_self function in the ArtefactType base class to stop view/artefact.php from breaking (personally i prefer this option)

(2) Make the title link to view/artefact.php optional in the render_viewing function in blocktype/lib.php. We could allow blocktypes to say whether to add the link or not, or perhaps control it at the instance config level so that blocktypes can allow the user to decide whether to link the title or not. (We should also allow view owners to turn off the display of the block titles altogether if they want to...)

Changed in mahara:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
Eugene (eugenev) wrote :

Richard - Please see the patch I'm attaching herewith.

Instead of creating a 'render_self' abstract method or implementing the method in the ArtefactType base class, the patch checks whether the 'render_self' method exists within the relevant artefact class, before creating the link to view.

I have also added a 'Display Title' option that allows for turning on/off the displaying of the title for a relevant block.

Please have a look and let me know if I misunderstood or if anything needs changing ;)

Revision history for this message
Richard Mansfield (richard-mansfield) wrote :

Eugene,

We always need to have render_self implemented for html export to work at all, otherwise you get a fatal error. So there's no point in checking for it with method_exists; it's better to write a dummy render_self function in the ArtefactType base class that prints the artefact title & description inside divs (or make it abstract).

I do think the titles should be optional though.

Revision history for this message
Eugene (eugenev) wrote :

Hi Richard,

Oh yea sorry, missed the part about the exports ;).
I have now made changes according to your suggestions (see the new patch). Please have a look and feel free to let me know if anything needs changing :).

Revision history for this message
Richard Mansfield (richard-mansfield) wrote :

Eugene,

Cool, allowing blocktypes to disable the link is perfect. I committed that part of your patch (http://gitorious.org/mahara/mahara/commit/e006bdf93ef0d9a3df3dead181a64a8a50badc56).

But I just realised that allowing instances to turn the title off needs more thought, so I'm going to open another bug for that: https://bugs.launchpad.net/mahara/+bug/655443

Changed in mahara:
milestone: none → 1.4.0
assignee: nobody → Eugene (eugene-catalyst)
status: Confirmed → Fix Committed
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.