Modify

Opened 3 years ago

Closed 5 months ago

#9210 closed defect (fixed)

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

Reported by: rjollos Owned by: hasienda
Priority: normal Component: TagsPlugin
Severity: normal Keywords: navigation
Cc: hasienda 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 rjollos 3 years ago.
BackToTicket.png (4.9 KB) - added by rjollos 3 years ago.
th9210-r10738-0.11.7-1.patch (810 bytes) - added by rjollos 3 years ago.
th9210-r10751-0.11.7.patch (981 bytes) - added by rjollos 3 years ago.

Download all attachments as: .zip

Change History (18)

comment:1 follow-up: Changed 3 years ago by hasienda

  • 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 3 years ago by rjollos

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 3 years ago by hasienda

Sure, a patch would be appreciated.

comment:4 Changed 3 years ago by rjollos

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 3 years ago by rjollos

Changed 3 years ago by rjollos

Changed 3 years ago by rjollos

comment:5 follow-up: Changed 3 years ago by hasienda

  • Owner changed from rjollos to hasienda
  • Status changed from new to assigned

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 follow-up: Changed 3 years ago by hasienda

  • Owner changed from hasienda to rjollos
  • Status changed from assigned to new

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

comment:7 in reply to: ↑ 6 Changed 3 years ago by rjollos

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 3 years ago by hasienda

  • Owner changed from rjollos to hasienda
  • Status changed from new to assigned

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 3 years ago by hasienda

(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 3 years ago by hasienda

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

comment:11 in reply to: ↑ 5 Changed 3 years ago by rjollos

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 3 years ago by rjollos

comment:12 Changed 3 years ago by hasienda

I like the latest improvement, thanks.

comment:13 Changed 3 years ago by hasienda

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

Done as suggested by Ryan J. Ollos, thanks.

comment:14 Changed 5 months ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to closed

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.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.