[JENKINS-68986] Remove link element from breadcrumb items without href

[janfaracik]

See JENKINS-68986.

Fairly straightforward PR to remove link elements from breadcrumb items without href set, as seen in 68986 - thanks @daniel-beck.

To make the breadcrumb bar experience more consistent, I've removed the custom breadcrumb implementation from lines 45 - 57 in favour of using the breadcrumb.jelly component. This means that we only have to include logic/customisation in one place. If anyone has any issues with this implementation/finds bugs do let me know.

Before - notice the hover state

After - notice the lack of hover state/inability to click the breadcrumb item

Proposed changelog entries

• Do not include links on bread crumb elements that do not define a hyperlink destination.

Proposed upgrade guidelines

N/A

Submitter checklist

• (If applicable) Jira issue is well described
• Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change) and are in the imperative mood. Examples
  • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
• Appropriate autotests or explanation to why this change has no tests
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadoc, as appropriate.
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO") if applicable.
• For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@jenkinsci/sig-ux @daniel-beck

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least 2 approvals for the pull request and no outstanding requests for change
• Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
• Changelog entries in the PR title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood
• Proper changelog labels are set so that the changelog can be generated automatically
• If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

wrong

[daniel-beck]

wrong

I thought my approval was premature and I had discovered a problem, but that was incorrect.

[timja]

Works for the manual ones at least.

There's a number of other ones that could have their links removed, like the CLI (e.g. /manage/cli/)

[timja]

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

[daniel-beck]

There's a number of other ones that could have their links removed, like the CLI (e.g. /manage/cli/)

I just re-ran the PR build and the "Jenkins CLI" crumb works.

Every automatic crumb has a URL (by convention, ModelObject doesn't have #getUrl) so wouldn't lose the link. If there's no anc.url, the (AFAICT, untested) behavior of linking to / remains unchanged.

[timja]

linking to / remains unchanged

correct this has no change over the previous behaviour, just something I noticed.

Every automatic crumb has a URL (by convention, ModelObject doesn't have #getUrl) so wouldn't lose the link. If there's no anc.url, the (AFAICT, untested) behavior of linking to / remains unchanged.

What I mean is you can click the CLI crumb currently, in this PR it removes the link state etc around the crumbs for the ones without links.

There's no reason to have a link on the active crumb so that could be removed (and likely should be emphasized to make it look different as the active crumb too).

[daniel-beck]

What I mean is you can click the CLI crumb currently, in this PR it removes the link state etc around the crumbs for the ones without links.

Not for me. Could you provide screenshots of what specifically you're referring to?

[janfaracik]

If we wanted to remove the link for the current page, we could do something like -

<j:choose>
        <j:set var="isCurrent" value="${h.hyperlinkMatchesCurrentPage(attrs.href)}" />
        <j:when test="${attrs.href == null or isCurrent}">
          ${attrs.title}
        </j:when>
        <j:otherwise>
          <a href="${attrs.href}" class="${attrs.hasMenu ? 'model-link' : ''}">
            ${attrs.title}
          </a>
        </j:otherwise>
      </j:choose>

[timja]

I think that would make sense for consistency. (also I think it would be good to show the difference between non linked breadcrumbs and linked ones maybe an underline)

[timja]

What I mean is you can click the CLI crumb currently, in this PR it removes the link state etc around the crumbs for the ones without links.

Not for me. Could you provide screenshots of what specifically you're referring to?

Links to current page:

No link:

[daniel-beck]

@timja I see, I misunderstood your comment to mean this PR already changes things, when it doesn't, but could.

Better off in a separate PR, but probably useful.

FWIW /configure vs. /configureTools is a fairly good example demonstrating the problem from a user POV.