Milestones should have a 'Back to Roadmap' link in the contextual navigation

[Ryan J Ollos]

When navigating to a milestone from the roadmap, there should be a Back to Roadmap link displayed in the contextual navigation menu.

This is not currently implemented in Trac 0.12dev, as can be seen: milestone:0.12-multirepos

This seems simple enough that I'll try to implemented it and provide a patch.

[Remy Blank]

I guess the reason why there is no such link is that you can just click on "Roadmap". However, I always have to think twice before doing that, because I'm used to the "Back to …" links in the ctxtnav. So I'm +1 for adding that link.

Bonus points if you implement it so that:

• The link leads back to the same milestone in the roadmap. You will need to add anchors in the roadmap, as there are currently none.

• The "Show already completed milestones" checkbox is selected if the milestone is closed.

Scheduling for next-minor-0.12.x until a patch is available.

[Christian Boos]

We had a "mid-air collision" :-)

---

There's already the main navigation item Roadmap for this, which moreover is highlighted when you're looking at Milestone, so I don't see what a Back to Roadmap contextual link would add.

Generally speaking, the contextual "Back" is used when looking at a resource from a particular selection (e.g. a ticket from a query or report), or a child resource (e.g. back to the parent resource from an attachment).

---

But yes, going back to the actual position of the milestone within the roadmap and activating Show closed milestones when seeing that "#themilestone" is actually closed could be an added value justifying the feature…

[anonymous]

Good points … I see now why this wasn't implemented already. I have the ​th:MenusPlugin installed so the Roadmap link is buried in a submenu, and I usually navigate via the contextual navigation menu, which is why I was expecting a link to be there.

I will work on implementing it with the proposed features (navigate back to specific milestone incl a closed milestone), but probably won't get to this until next weekend.

We have a lot of milestones in my Trac instance, so I really like the idea of navigating back to a specific milestone in the roadmap.

[ryano@…]

Whoops … Should have logged in before posting that comment, but it was probably obvious that the comment was from the reporter.

[Ryan J Ollos <ryano@…>]

I have a working version of a patch that still has at least some minor issues to be worked out, and I'd really appreciate some feedback on it.

I'm storing the roadmap filter state in the req.session, so that it behaves more like the timeline filter with regard to remembering a user's selection. Now, one can navigate away from the roadmap page, and when returning to the roadmap page the selection for Show completed milestones will still have the same value. Previously it would always be unselected. After the patch in #9036 is applied, I'll need to modify this patch to support the second filter.

There are currently two issues:

1. If I navigate to a roadmap entry, e.g. Develop project plan, the request is for /milestone/Develop project plan. I select Back to Roadmap and the request is for /roadmap#Develop project plan. So far, things seem fine. Now I scroll up to the preferences box, check Show completed milestones and press update; the request is for /roadmap?showcompleted=on&update=Update#Develop project plan. It seems to work, but I was expecting something more like /roadmap#Develop project plan?showcompleted=on&update=Update.
2. The anchor appears on a separate line from the milestone heading as shown in the image below.

This is the biggest task I have tried to do in python so far, so I'm prepared for the fact that this might need significantly more work.

[Christian Boos]

Replying to Ryan J Ollos <ryano@…>:

… the request is for /roadmap?showcompleted=on&update=Update#Develop project plan. It seems to work, but I was expecting something more like /roadmap#Develop project plan?showcompleted=on&update=Update.

This is how it's supposed to work (anchor has to come after the request parameters, see ​rfc:2396#section-4).

1. The anchor appears on a separate line from the milestone heading as shown in the image below.

This is indeed problematic. Maybe look if there's any structural differences in the resulting HTML between this and e.g. the Wiki section links. If not, then try to identify the CSS differences (the Web Inspector in Chrome or Firebug in Firefox are invaluable tools for troubleshooting this kind of annoyance).

Otherwise the patch looks quite good. I'll add a few comments about the parts that can nevertheless be improved later today. Keep up the good work!

[Christian Boos]

Btw, the highlighting idea of ticket:7573#comment:12 would apply here as well.

[anonymous]

The Back to Roadmap contextual navigation link was added in #9820 / [10321]. There are a couple of additional features that could be addressed in this ticket, as noted in comment:1.

[anonymous]

The patch has been refreshed and now provides the following features:

• The Back to roadmap link on the milestone page leads back to the same milestone in the roadmap.
• Milestones on the roadmap have anchors.
• The milestone is highlighted on the roadmap (comment:7) when navigating to a specific milestone.
• The Show completed milestones milestones is selected if the milestone is closed. Unlike the earlier path, the roadmap filter states are not stored in the session data.

The patch also fixes some coding style issues for readable strings (replacing single quotes with double quotes). The functional tests have been executed and pass, although this patch does not add any functional tests. I'd be happy to add some if the patch is deemed acceptable and will be committed.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Christian Boos]

Looks good! Thanks for the patch, I'll test later today.

[Remy Blank]

I believe the anchor of the href for the up link is missing some form of URL-encoding, as it's added outside of the ref.href.roadmap() call.

[Ryan J Ollos <ryan.j.ollos@…>]

I'll have to look more closely, but it looks like maybe I should be calling trac.util.text.unicode_quote. What do you thinking about adding support to the Href class for either handling a kw argument named fragment, or special handling of string arguments that begin with #?

[Christian Boos]

The fragment keyword argument is an interesting idea, though we currently don't do much normalization of the fragment. And this doesn't seem to be a problem in practice, as even the validators don't seem to bother much about the precise rules for href fragments.

For example, even an URL ending with /roadmap#123#繁體中文 works fine for IE[7:10], FF, Opera and Chrome, the W3C XHTML validator doesn't complain, neither does LXML. So I don't think we should be more strict than these tools are.

OTOH, we may want to be stricter for what we put in the id attribute, as there we have to be more careful if we want to remain XHTML 1.0 compliant. But maybe our efforts should be better spent in migrating towards HTML5 anyway… which I suppose is already the approach you opted for, with replace(' ', ''). That might go in a helper function regardless what we decide (trac.util.html.normalize_id?).

[Ryan J Ollos <ryan.j.ollos@…>]

It sounds like I should do the following then:

• Add a new function trac.util.html.normalize_id which for now only calls replace(' ', '').
• Call trac.util.html.normalize_id in the two places that I currently call replace(' ', '').

Should I pass trac.util.html.normalize_id in the data for this template, or add it to the default context data?

[Christian Boos]

Replying to Ryan J Ollos <ryan.j.ollos@…>:

It sounds like I should do the following then:

Fine by me.

Should I pass trac.util.html.normalize_id in the data for this template, or …

Or pass the html module itself, perhaps as util_html (or instead of .html, put it in .presentation and pass .presentation itself?).

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Ryan J Ollos <ryan.j.ollos@…>]

Patch against r11483 of the trunk.

[Ryan J Ollos <ryan.j.ollos@…>]

t8989-r11483-2.patch​ has the changes discussed, but puts presentation in the default context data. If we go with this patch, then there are 7 functions​ from presentation that are passed individually. Would it be worthwhile to deprecate the use of those 7 functions and make changes through the codebase to use presentation.<function> rather than <function>?

t8989-r11483-3.patch​ has the changes discussed and puts presentation in the roadmap template data.

[anonymous]

Hm, why add a second Roadmap link to the page, directly under the existing Roadmap link, with exactly the same functionality?

If it were a history.back(-1) or history.back(-2) then it would make sense as the user agent would bring me back to the same scroll offset in the road map's milestone list.

Right now, using the back button seems more useful to me.

[anonymous]

Replying to anonymous:

Ups, just saw that it is not in the T.E.O. version but that work is on the way to make this different from the original Roadmap link.

[Ryan J Ollos]

Narrowing focus for milestone:1.2. Please move ticket to milestone:1.2 if you intend to fix it.

[Ryan J Ollos]

Milestone renamed

[Ryan J Ollos]

Milestone renamed