Breadcrumbs display for IHasMajorHeading views

Bug #433852 reported by Michael Nelson
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Undecided
Michael Nelson

Bug Description

According the the current heading rules at:

https://dev.launchpad.net/VersionThreeDotO/UI/Conversion#Heading rules

for pages where the context is the root context, breadcrumbs should not display.

The url:

https://launchpad.dev/sprints/futurista

is a bit strange, in that the sprint content class does provide IRootContext, and so the heading is displayed in the watermark. Adding IMajorHeadingView to the SprintView then correctly ensures that the heading is displayed as an H1 - but the breadcrumbs still display below the application tabs. (Note, we don't normally see this, for example, for people, because there are not normally > 1 breadcrumbs, but in this case there is one for the ISprintSet and then the index.)

Barry provided a work-around for this, which was to provide a SprintIndexHierarchy which has display_breadcrumbs set to False - and this fixes the issue on the sprint index, but it also ensures that sub-pages no-longer display breadcrumbs (as in the work-around the SprintIndexHierarchy is a hierarchy view used for ISprint generally so all the sub-pages which have the same context get display_breadcrumbs == False too).

Another work-around would be to disable/remove the ISprintSet breadcrumb - I'm not sure what would be better. I'll take a look at what would be involved to fix the actual bug too.

Related branches

description: updated
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Here's a patch - I'll get salgado to take a look, and if it's ok, land it with my branch.

Changed in launchpad-foundations:
status: New → In Progress
assignee: nobody → Michael Nelson (michael.nelson)
Revision history for this message
Barry Warsaw (barry) wrote :

Your patch looks good, thanks for fixing this and thanks for the test. I made a few minor style comments in irc which you've fixed.

(I suggested moving the import of removeSecurityProxy to the module globals, but Salgado provided a good rationale for why we shouldn't do that)

s/_context_view/_naked_context_view

split the return into multiple lines, such as:

    has_major_heading = IMajorHeading.providedBy(self._naked_context_view)
    return len(self.items) > 1 and not has_major_heading

Revision history for this message
Michael Nelson (michael.nelson) wrote :

r9557

Changed in launchpad-foundations:
status: In Progress → Fix Committed
Changed in launchpad-foundations:
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.