[ITB-2164] Treat navigation controls (e.g., menu, buttons, table rows) as links allowing opening in new tabs

Author: costas80

gitb-ui/ui/src/app/app.module.ts

@@ -295,6 +295,7 @@ import {StartupWizardModalComponent} from './modals/startup-wizard-modal/startup
 import {RadioCardComponent} from './components/radio-card/radio-card.component';
 import {UsageTipModalComponent} from './modals/usage-tip-modal/usage-tip-modal.component';
 import {SubmitOnControlEnterDirective} from './directives/submit-on-control-enter.directive';
+import {NavTargetDirective} from './directives/nav-target.directive';
 import {
   NgbAlertModule,
   NgbCollapseModule,
@@ -541,7 +542,8 @@ import { TestResultCommentsModalComponent } from './modals/test-result-comments-
         NgbNavModule,
         CodemirrorModule,
         MarkdownModule.forRoot(),
-        SubmitOnControlEnterDirective
+        SubmitOnControlEnterDirective,
+        NavTargetDirective
     ], providers: [
         CookieService,
         { provide: HUGERTE_SCRIPT_SRC, useValue: 'hugerte/hugerte.min.js' },

gitb-ui/ui/src/app/common/utils.ts

@@ -89,4 +89,16 @@ export class Utils {
       return removed;
     }
 
+    /**
+     * Determines whether a click on a navigation link (`<a [navTarget]>`) is a plain, same-tab
+     * navigation as opposed to one that opens the link elsewhere (a modifier click, or a
+     * non-primary mouse button). Used to guard side effects that should only apply to the current
+     * tab (e.g. recording a "return to source" location, or clearing display state) - since a
+     * modified click still fires the DOM "click" event even though the browser opens the link in
+     * a new tab/window rather than navigating away from the current page.
+     */
+    public static isPlainNavigationClick(event: MouseEvent): boolean {
+        return event.button === 0 && !event.ctrlKey && !event.metaKey && !event.shiftKey && !event.altKey
+    }
+
 }

gitb-ui/ui/src/app/components/base-table/base-table.component.ts

@@ -22,6 +22,7 @@ import {TableRowApi} from '../table-row/table-row-api';
 import {Constants} from '../../common/constants';
 import {Observable} from 'rxjs';
 import {CheckboxOption} from '../checkbox-option-panel/checkbox-option';
+import {NavigationTarget} from '../../types/navigation-target';
 
 @Component({
     template: '',
@@ -64,6 +65,7 @@ export abstract class BaseTableComponent extends BaseComponent {
   @Input() optionProvider?: (row: any) => Observable<CheckboxOption[][]>
   @Input() optionsVisibleForRow?: (row: any) => boolean
   @Input() optionPendingProperty = 'optionPending'
+  @Input() rowTarget?: (row: any) => NavigationTarget|undefined
 
   @Output() onSelect: EventEmitter<any> = new EventEmitter()
   @Output() onDeselect: EventEmitter<any> = new EventEmitter()
@@ -74,6 +76,7 @@ export abstract class BaseTableComponent extends BaseComponent {
   @Output() onDelete: EventEmitter<any> = new EventEmitter()
   @Output() pageNavigation: EventEmitter<PagingEvent> = new EventEmitter()
   @Output() onSort: EventEmitter<TableColumnDefinition> = new EventEmitter()
+  @Output() navigating: EventEmitter<MouseEvent> = new EventEmitter()
 
   @ViewChildren("tableRowComponent") tableRowComponents?: QueryList<TableRowApi>
 
@@ -136,6 +139,10 @@ export abstract class BaseTableComponent extends BaseComponent {
     this.pageNavigation.emit(event)
   }
 
+  propagateNavigating(event: MouseEvent) {
+    this.navigating.emit(event)
+  }
+
   @HostListener('document:click', ['$event'])
   clickRegistered(event: Event) {
     this.tableRowComponents?.forEach((component) => component.documentClick(event))

gitb-ui/ui/src/app/components/breadcrumb/breadcrumb-item.ts

@@ -14,11 +14,14 @@
  */
 
 import { BreadcrumbType } from "src/app/types/breadcrumb-type"
+import { NavigationTarget } from "src/app/types/navigation-target"
 
 export interface BreadcrumbItem {
 
     type: BreadcrumbType
     action?: Function
+    /** Populated alongside `action` whenever the crumb navigates, so it can be rendered as a real link. */
+    target?: NavigationTarget
 
     label?: string
     typeId?: number|string

gitb-ui/ui/src/app/components/breadcrumb/breadcrumb.component.html

@@ -6,17 +6,30 @@
                     @if (!$first) {
                         <div class="breacrumbItemSeparator"><i class="fa-solid fa-angle-right"></i></div>
                     }
-                    <div class="breacrumbItem"
-                        [class.actionable]="crumb.action != undefined"
-                        #popup="ngbTooltip"
-                        (mouseover)="mouseOver($event)"
-                        [tooltipClass]="'shortTooltip'"
-                        placement="bottom"
-                        [ngbTooltip]="crumb.label"
-                        [openDelay]="Constants.TOOLTIP_DELAY"
-                        [disableTooltip]="!hasOverflow"
-                        (click)="popup.close();breadcrumbClicked(crumb)"
-                    >{{crumb.label}}</div>
+                    @if (crumb.target) {
+                        <a class="breacrumbItem actionable plain-link"
+                            #popup="ngbTooltip"
+                            (mouseover)="mouseOver($event)"
+                            [tooltipClass]="'shortTooltip'"
+                            placement="bottom"
+                            [ngbTooltip]="crumb.label"
+                            [openDelay]="Constants.TOOLTIP_DELAY"
+                            [disableTooltip]="!hasOverflow"
+                            [navTarget]="crumb.target"
+                            (click)="popup.close();breadcrumbClicked($event)"
+                        >{{crumb.label}}</a>
+                    } @else {
+                        <div class="breacrumbItem"
+                            #popup="ngbTooltip"
+                            (mouseover)="mouseOver($event)"
+                            [tooltipClass]="'shortTooltip'"
+                            placement="bottom"
+                            [ngbTooltip]="crumb.label"
+                            [openDelay]="Constants.TOOLTIP_DELAY"
+                            [disableTooltip]="!hasOverflow"
+                            (click)="popup.close()"
+                        >{{crumb.label}}</div>
+                    }
                 }
             }
         }

gitb-ui/ui/src/app/components/breadcrumb/breadcrumb.component.less

@@ -26,6 +26,12 @@
         margin: 0px 3px;
     }
     .breacrumbItem {
+        /*
+         * Explicit even though flex-item blockification already achieves this for the <a> crumbs
+         * (actionable crumbs are now rendered as anchors so the browser exposes normal link
+         * behaviour - open in new tab, copy link, etc.) - kept for clarity/defensiveness.
+         */
+        display: block;
         padding: 1px 5px;
         margin: 0px 2px;
         border-radius: @border-radius;

gitb-ui/ui/src/app/components/breadcrumb/breadcrumb.component.ts

@@ -22,6 +22,7 @@ import {BreadcrumbService} from 'src/app/services/breadcrumb.service';
 import {BreadcrumbLabelRequest} from 'src/app/types/breadcrumb-label-request';
 import {Subscription} from 'rxjs';
 import {Constants} from 'src/app/common/constants';
+import {Utils} from 'src/app/common/utils';
 
 @Component({
     selector: 'app-breadcrumb',
@@ -51,7 +52,8 @@ export class BreadcrumbComponent implements OnInit, OnDestroy {
     this.homeCrumb = {
       label: 'Home',
       type: BreadcrumbType.home,
-      action: () => this.routingService.toHome()
+      action: () => this.routingService.toHome(),
+      target: this.routingService.linkToHome()
     }
     this.breadcrumbSubscription = this.dataService.onBreadcrumbChange$.subscribe((info) => {
       setTimeout(() => {
@@ -168,10 +170,15 @@ export class BreadcrumbComponent implements OnInit, OnDestroy {
     }
   }
 
-  breadcrumbClicked(crumb: BreadcrumbItem) {
-    if (crumb.action) {
+  /**
+   * Navigation itself is now handled by the crumb's own [navTarget] (a real router link) - this
+   * only clears display state, and only for a plain (unmodified, primary-button) click: a
+   * ctrl/cmd/shift/alt click or middle-click still fires this "click" handler even though the
+   * crumb opens in a new tab/window rather than navigating away from the current one.
+   */
+  breadcrumbClicked(event: MouseEvent) {
+    if (Utils.isPlainNavigationClick(event)) {
       this.dataService.clearAllDisplayStates()
-      crumb.action()
     }
   }
 

gitb-ui/ui/src/app/components/checkbox-option-panel/checkbox-option-panel.component.html

@@ -10,18 +10,26 @@
                 @for (optionSet of options; track index1; let index1 = $index) {
                     <div class="row">
                         @for (option of optionSet; track index2; let index2 = $index) {
-                            <div class="col-12 itemContainer" (click)="handleClick(option.key); $event.stopPropagation()">
-                                <div class="d-flex">
-                                    @if (singleSelection) {
+                            @if (option.target) {
+                                <a class="col-12 itemContainer plain-link" [navTarget]="option.target" (click)="handleClick(option, $event); $event.stopPropagation()">
+                                    <div class="d-flex">
                                         <div class="optionLabel">@if (option.iconClass) { <div class="optionIcon"><i [ngClass]="option.iconClass!"></i></div> } <div>{{option.label}}</div></div>
-                                    } @else {
-                                        <input class="form-check-input" type="checkbox" [attr.id]="'option-'+index1+'-'+index2" [attr.name]="'option-'+index1+'-'+index2" [(ngModel)]="currentState[option.key]" (ngModelChange)="updated.emit(currentState)" [disabled]="option.disabled || false">
-                                        <label class="form-check-label disable-select" [attr.for]="'option-'+index1+'-'+index2" [class.text-muted]="option.disabled">
-                                            <span class="optionLabel">@if (option.iconClass) { <i class="optionIcon" [ngClass]="option.iconClass!"></i> } {{option.label}}</span>
-                                        </label>
-                                    }
+                                    </div>
+                                </a>
+                            } @else {
+                                <div class="col-12 itemContainer" (click)="handleClick(option); $event.stopPropagation()">
+                                    <div class="d-flex">
+                                        @if (singleSelection) {
+                                            <div class="optionLabel">@if (option.iconClass) { <div class="optionIcon"><i [ngClass]="option.iconClass!"></i></div> } <div>{{option.label}}</div></div>
+                                        } @else {
+                                            <input class="form-check-input" type="checkbox" [attr.id]="'option-'+index1+'-'+index2" [attr.name]="'option-'+index1+'-'+index2" [(ngModel)]="currentState[option.key]" (ngModelChange)="updated.emit(currentState)" [disabled]="option.disabled || false">
+                                            <label class="form-check-label disable-select" [attr.for]="'option-'+index1+'-'+index2" [class.text-muted]="option.disabled">
+                                                <span class="optionLabel">@if (option.iconClass) { <i class="optionIcon" [ngClass]="option.iconClass!"></i> } {{option.label}}</span>
+                                            </label>
+                                        }
+                                    </div>
                                 </div>
-                            </div>
+                            }
                         }
                         @if (!$last) {
                             <div class="separator"></div>

gitb-ui/ui/src/app/components/checkbox-option-panel/checkbox-option-panel.component.less

@@ -62,7 +62,11 @@ label {
     white-space: nowrap;
 }
 .row {
-    & > div {
+    /*
+     * Matches both <div> (action options) and <a> (options that navigate, rendered as a real link -
+     * see CheckboxOption.target) item containers.
+     */
+    & > div, & > a {
         padding: 8px 20px 8px 14px;
     }
 }

gitb-ui/ui/src/app/components/checkbox-option-panel/checkbox-option-panel.component.ts

@@ -59,6 +59,8 @@ export class CheckboxOptionPanelComponent implements OnInit, OnDestroy, CheckBox
   @Output() opening = new EventEmitter<void>()
   @Output() opened = new EventEmitter<void>()
   @Output() closed = new EventEmitter<void>()
+  /** Emitted for options with a `target` (navigation happens via the option's own [navTarget]) so a caller can still run pre-navigation side effects (e.g. recording a "return to source" location). */
+  @Output() navigate = new EventEmitter<MouseEvent>()
 
   @ViewChild("button") buttonElement?: ElementRef<HTMLButtonElement>
   @ViewChild('popupTemplate') popupTemplate?: TemplateRef<any>;
@@ -273,13 +275,23 @@ export class CheckboxOptionPanelComponent implements OnInit, OnDestroy, CheckBox
     }
   }
 
-  handleClick(key: string) {
+  /**
+   * For an option with a `target`, navigation happens via the option's own [navTarget] (a real
+   * router link) rather than here - emitting `updated` for it too would cause a second, imperative
+   * navigation on top of the link's own. Such options only need the popup closed, and give the
+   * caller a chance (via `navigate`) to run any pre-navigation side effect.
+   */
+  handleClick(option: CheckboxOption, event?: MouseEvent) {
     if (this.singleSelection) {
-      this.currentState = {}
-      this.currentState[key] = true
-      const event:CheckboxOptionState = {}
-      event[key] = true
-      this.updated.emit(event)
+      if (option.target == undefined) {
+        this.currentState = {}
+        this.currentState[option.key] = true
+        const emitted: CheckboxOptionState = {}
+        emitted[option.key] = true
+        this.updated.emit(emitted)
+      } else if (event != undefined) {
+        this.navigate.emit(event)
+      }
       this.close()
     }
   }