Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-44522

Top-level navigation does not show selected state

    Details

    • Epic Link:
    • Sprint:
      Blue Ocean 1.2-beta1, Blue Ocean 1.2-beta2
    • Similar Issues:

      Description

      The Blue Ocean interface does not display a selected state for the top-level navigation. This is especially problematic when plugins add additional top-level links to the interface. Because react renders so quickly it can create a usability problem where the user does not recognize that they have navigated to a new page because the links to do not transition to a "selected" state.

      The correct selected state is pictured in the attached screenshot (navigation-selected-state.png) and is already present in the CSS. In fact it's used for run result pages already. We just need to teach React about it for the top links.

       

       

        Attachments

          Issue Links

            Activity

            Hide
            sophistifunk Josh McDonald added a comment -

            Agreed. Also worried that if we put in a bunch of kludgey stuff early, it'll get left in  That being said, we needn't take a bunch of steps right now to worry about what redirects may be done by future extensions, but rather just the current (and hardcoded IIRC) redirect that we have for '/'

            Show
            sophistifunk Josh McDonald added a comment - Agreed. Also worried that if we put in a bunch of kludgey stuff early, it'll get left in  That being said, we needn't take a bunch of steps right now to worry about what redirects may be done by future extensions, but rather just the current (and hardcoded IIRC) redirect that we have for '/'
            Hide
            cliffmeyers Cliff Meyers added a comment -

            I fiddled with this for a while last week and doing this off URL-based matching is a bit of a mess. I just don't think it's the right way to do it. Rather we might want to consider a short-term solution along these lines:

            1. Plugin defines some kind of POJO which holds state as to whether it's active or not,
            2. The "Link" component added to the top level nav requires this component and reads its active state
            3. The route definition(s) from the plugin use the onChange handler to manipulate the state of that POJO as active or inactive.

            Later on, with typed extension points, this behavior could probably be baked in nearly 100%... but things will still get 'weird" if you have plugins that start to add routes that are nested underneath other routes from another plugin (say adding a tab to run details).

            Per James Dumay I have some bigger fish to fry at the moment, although if I can crank out the above solution in hour or so perhaps I'll dive right in and do it.

            Show
            cliffmeyers Cliff Meyers added a comment - I fiddled with this for a while last week and doing this off URL-based matching is a bit of a mess. I just don't think it's the right way to do it. Rather we might want to consider a short-term solution along these lines: Plugin defines some kind of POJO which holds state as to whether it's active or not, The "Link" component added to the top level nav requires this component and reads its active state The route definition(s) from the plugin use the onChange handler to manipulate the state of that POJO as active or inactive. Later on, with typed extension points, this behavior could probably be baked in nearly 100%... but things will still get 'weird" if you have plugins that start to add routes that are nested underneath other routes from another plugin (say adding a tab to run details). Per James Dumay I have some bigger fish to fry at the moment, although if I can crank out the above solution in hour or so perhaps I'll dive right in and do it.
            Hide
            cliffmeyers Cliff Meyers added a comment -

            James Dumay I know you asked me to deprioritize this ticket, but I had an idea and was able to crank this out in about an hour... so I figured I'd throw the PR up and see what the thoughts are. If it gets thorny I'll just press pause on this thing for a while and fry some bigger fish. At least now if someone wants to tackle this bug they can perhaps make use of this code.

            Show
            cliffmeyers Cliff Meyers added a comment - James Dumay I know you asked me to deprioritize this ticket, but I had an idea and was able to crank this out in about an hour... so I figured I'd throw the PR up and see what the thoughts are. If it gets thorny I'll just press pause on this thing for a while and fry some bigger fish. At least now if someone wants to tackle this bug they can perhaps make use of this code.
            Hide
            jlong John Long added a comment -

            Cliff Meyers can you link the pull request. Just curious about the approach.

            Show
            jlong John Long added a comment - Cliff Meyers can you link the pull request. Just curious about the approach.
            Hide
            cliffmeyers Cliff Meyers added a comment -

            John Long see link above.

            Show
            cliffmeyers Cliff Meyers added a comment - John Long see link above.

              People

              • Assignee:
                cliffmeyers Cliff Meyers
                Reporter:
                jlong John Long
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: