Modify

Opened 6 years ago

Closed 3 years ago

#9210 closed defect (fixed)

/tags page should not have a contextual navigation link 'Cloud'

Reported by: Ryan J Ollos Owned by: Steffen Hoffmann
Priority: normal Component: TagsPlugin
Severity: normal Keywords: navigation
Cc: Steffen Hoffmann Trac Release: 0.11

Description

The /tags page has a contextual navigation link Cloud. However, this link just points to /tags, therefore it offers no functionality when on the /tags page ... unless I am missing something?

It seems like the contextual navigation link should not be displayed when the /tags page is displayed. I also question it's value when after a tags query has been submitted. It allows navigating back to the /tags page, but one can also just click on the Tags entry on the mainnav.

Unless I'm missing something, it seems like the Cloud contextual nav entry is just cluttering up the contextual navigation.

Attachments (4)

BackToQuery.png (8.7 KB) - added by Ryan J Ollos 6 years ago.
BackToTicket.png (4.9 KB) - added by Ryan J Ollos 6 years ago.
th9210-r10738-0.11.7-1.patch (810 bytes) - added by Ryan J Ollos 6 years ago.
th9210-r10751-0.11.7.patch (981 bytes) - added by Ryan J Ollos 6 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 6 years ago by Steffen Hoffmann

Keywords: navigation added

While initially I'd agree, clutter=defect is a bit harsh, and how about non-standard configuration like

[mainnav]
tags.href= /tags?q=Test

so the ctxtnav entry would remain as the only way to quickly revert to tag cloud?

Still for the majority of installations the obsolete redundancy may be a valid claim. What do others think? At least I don't feel a strong need for yet another option here, right?

A bit far-fetched for such a small link - anyway: Should we detect tags.href<>"/tags" and hide/unhide that ctxtnav link accordingly?

comment:2 in reply to:  1 Changed 6 years ago by Ryan J Ollos

Replying to hasienda:

While initially I'd agree, clutter=defect is a bit harsh ...

Agreed ... though it seemed a contradiction to call it an enhancement, even though that is probably how it is best described - enhance by removing clutter ;)

A bit far-fetched for such a small link - anyway: Should we detect tags.href<>"/tags" and hide/unhide that ctxtnav link accordingly?

Yeah, that was one thought that I had, and would seem to handle the mainnav configuration case you mentioned. I think I could handle providing that patch ... I did a similar patch for the Trac core a while back.

comment:3 Changed 6 years ago by Steffen Hoffmann

Sure, a patch would be appreciated.

comment:4 Changed 6 years ago by Ryan J Ollos

Okay, here's a patch. Looking forward to your feedback!

Another idea would be to rename the Cloud metanav link to Back to TagCloud, for consistency with other "child" pages in Trac.

Similar to:

Changed 6 years ago by Ryan J Ollos

Attachment: BackToQuery.png added

Changed 6 years ago by Ryan J Ollos

Attachment: BackToTicket.png added

Changed 6 years ago by Ryan J Ollos

comment:5 Changed 6 years ago by Steffen Hoffmann

Owner: changed from Ryan J Ollos to Steffen Hoffmann
Status: newassigned

Simple but good idea, like it too. Let me sleep about it, then I may just do it that way.

Any chance to give me a hint on how you did the conditional hiding for Trac core you mentioned before? I guess, this could save me some time, but not a blocker for me either.

comment:6 Changed 6 years ago by Steffen Hoffmann

Owner: changed from Steffen Hoffmann to Ryan J Ollos
Status: assignednew

Sorry, didn't mean to take it away from you. :-)

comment:7 in reply to:  6 Changed 6 years ago by Ryan J Ollos

Replying to hasienda:

Sorry, didn't mean to take it away from you. :-)

Please feel free to reassign. I just assigned it to myself to let everyone know I intended to implement. Once my part is done, it's all yours ;)

I'll get back to you with regarding your other question by tonight.

comment:8 Changed 6 years ago by Steffen Hoffmann

Owner: changed from Ryan J Ollos to Steffen Hoffmann
Status: newassigned

I could do an additional check in like

    def process_request(self, req):
        req.perm.require('TAGS_VIEW')
        if 'q=' in self.env.config.get('mainnav', 'tags.href', '') and \
                'q' in req.args:
            # TRANSLATOR: The meta-nav link label.
            add_ctxtnav(req, _("Back to Cloud"), req.href.tags())

but I feel having the back-link isn't evil anymore, if we hide it when displaying the cloud.

comment:9 Changed 6 years ago by Steffen Hoffmann

(In [10748]) TagsPlugin: Hide 'Cloud' back-link, if showing TagCloud in '/tags', refs #9210.

Renaming the link to 'Back to Cloud' is more consistent with use of context navigation in Trac core (i.e. see TagsQuery). Thanks to Ryan for this idea.

comment:10 Changed 6 years ago by Steffen Hoffmann

(In [10751]) TagsPlugin: Hide 'Cloud' back-link on '/tags' page for empty tag queries too, refs #9210.

comment:11 in reply to:  5 Changed 6 years ago by Ryan J Ollos

Replying to hasienda:

Any chance to give me a hint on how you did the conditional hiding for Trac core you mentioned before? I guess, this could save me some time, but not a blocker for me either.

I wasn't able to find the ticket I was trying to think of. I might be remembering incorrectly.

There might be an opportunity here a small amount of refactoring to improve the existing patches. I'll attach a patch for that now.

Changed 6 years ago by Ryan J Ollos

Attachment: th9210-r10751-0.11.7.patch added

comment:12 Changed 6 years ago by Steffen Hoffmann

I like the latest improvement, thanks.

comment:13 Changed 6 years ago by Steffen Hoffmann

(In [10755]) TagsPlugin: Hide simplify check to not duplicate existing logic, refs #9210.

Done as suggested by Ryan J. Ollos, thanks.

comment:14 Changed 3 years ago by Steffen Hoffmann

Resolution: fixed
Status: assignedclosed

In 13815:

TagsPlugin: Completing preparation for v0.7 release.

Availability of that code as stable, tagged release
closes #2429, #3359, #3610, #3624, #3677, #3754, #3864, #3947, #3983, #4078, #4277, #4503, #4799, #5523, #7787, #7857, #8638, #9057, #9058, #9059, #9060, #9061, #9062, #9063, #9149, #9210, #9521, #9630, #9636, #10032, #10416, #10636, #11096, #11147, #11152, #11274, #11302, #11658 and #11659.

Additionally there are some issues and enhancement requests showing progress,
but known to require more work to resolve them satisfactorily, specifically
refs #2804, #4200, #8747 and #9064.

Thanks to all contributors and followers, that enabled and encouraged a good
portion of this development work.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.