Page header fixes

.changeset/grumpy-bananas-glow.md

@@ -0,0 +1,5 @@
+---
+"@openproject/primer-view-components": patch
+---
+
+Fine-tuning of PageHeader component (spacings, size, font-weight)

app/components/primer/open_project/page_header.html.erb

@@ -5,7 +5,7 @@
       <%= breadcrumbs %>
       <% if render_mobile_menu? %>
         <%= render(@mobile_action_menu) do |menu| %>
-          <% menu.with_show_button(icon: :"kebab-horizontal", "aria-label": @mobile_menu_label) %>
+          <% menu.with_show_button(icon: :"kebab-horizontal", size: :small, "aria-label": @mobile_menu_label) %>
           <% @desktop_menu_block.call(menu) unless @desktop_menu_block.nil? %>
         <% end %>
       <% end %>

app/components/primer/open_project/page_header.pcss

@@ -13,6 +13,7 @@
   flex-flow: row;
   justify-content: flex-end;
   align-items: center;
+  margin-bottom: var(--base-size-8);
 }
 
 .PageHeader-titleBar {
@@ -59,8 +60,6 @@
 .PageHeader-breadcrumbs {
   display: block;
   width: 100%;
-  margin-bottom: var(--base-size-8);
-  padding-bottom: var(--base-size-4);
 }
 
 .PageHeader-leadingAction {

app/components/primer/open_project/page_header.rb

@@ -47,6 +47,7 @@ class PageHeader < Primer::Component
       # Optional description below the title row
       renders_one :description, lambda { |**system_arguments|
         deny_tag_argument(**system_arguments)
+
         system_arguments[:tag] = :div
         system_arguments[:classes] = class_names(system_arguments[:classes], "PageHeader-description")
 
@@ -59,41 +60,46 @@ class PageHeader < Primer::Component
       renders_many :actions, types: {
         icon_button: lambda { |icon:, mobile_icon:, label:, scheme: DEFAULT_ACTION_SCHEME, **system_arguments|
           deny_tag_argument(**system_arguments)
-          system_arguments = set_action_arguments(system_arguments, scheme: scheme)
+
+          system_arguments = set_action_arguments(system_arguments, scheme: scheme, button_action: true)
+
           add_option_to_mobile_menu(system_arguments, mobile_icon, label, scheme)
 
           Primer::Beta::IconButton.new(icon: icon, "aria-label": label, **system_arguments)
         },
         button: lambda { |mobile_icon:, mobile_label:, scheme: DEFAULT_ACTION_SCHEME, **system_arguments|
           deny_tag_argument(**system_arguments)
-          system_arguments = set_action_arguments(system_arguments, scheme: scheme)
+
+          system_arguments = set_action_arguments(system_arguments, scheme: scheme, button_action: true)
+
           add_option_to_mobile_menu(system_arguments, mobile_icon, mobile_label, scheme)
 
           Primer::Beta::Button.new(**system_arguments)
         },
         zen_mode_button: lambda { |mobile_icon: Primer::OpenProject::ZenModeButton::ZEN_MODE_BUTTON_ICON, mobile_label: Primer::OpenProject::ZenModeButton::ZEN_MODE_BUTTON_LABEL, **system_arguments|
           deny_tag_argument(**system_arguments)
-          system_arguments = set_action_arguments(system_arguments, scheme: DEFAULT_ACTION_SCHEME)
+
+          system_arguments = set_action_arguments(system_arguments, scheme: DEFAULT_ACTION_SCHEME, button_action: true)
+
           add_option_to_mobile_menu(system_arguments, mobile_icon, mobile_label, DEFAULT_ACTION_SCHEME)
 
           Primer::OpenProject::ZenModeButton.new(**system_arguments)
         },
 
         link: lambda { |mobile_icon:, mobile_label:, scheme: DEFAULT_ACTION_SCHEME, **system_arguments|
           deny_tag_argument(**system_arguments)
-          system_arguments[:target] ||= "_top"
 
+          system_arguments[:target] ||= "_top"
           system_arguments = set_action_arguments(system_arguments, scheme: scheme)
+
           add_option_to_mobile_menu(system_arguments, mobile_icon, mobile_label, scheme)
 
           Primer::Beta::Link.new(**system_arguments)
         },
         # Should only be used rarely on a per-need basis
         text: lambda { |**system_arguments|
           system_arguments = set_action_arguments(system_arguments)
-
           system_arguments[:color] ||= :muted
-
           # Enforce that texts are hidden on mobile
           system_arguments[:display] = [:none, :flex]
 
@@ -102,7 +108,11 @@ class PageHeader < Primer::Component
         menu: {
           renders: lambda { |**system_arguments, &block|
             deny_tag_argument(**system_arguments)
+
             system_arguments[:menu_arguments] = set_action_arguments(system_arguments[:menu_arguments])
+            system_arguments[:button_arguments] ||= {}
+            system_arguments[:button_arguments][:data] ||= {}
+            system_arguments[:button_arguments][:data][:targets] = "page-header.actionItems"
 
             # Add the options individually to the mobile menu in the template
             @desktop_menu_block = block
@@ -115,9 +125,10 @@ class PageHeader < Primer::Component
             deny_tag_argument(**system_arguments)
 
             # The id will be automatically calculated for the trigger button, so we have to behave the same, for the mobile click to work
+            system_arguments[:button_arguments] ||= {}
             system_arguments[:button_arguments][:id] = "dialog-show-#{system_arguments[:dialog_arguments][:id]}"
+            system_arguments[:button_arguments] = set_action_arguments(system_arguments[:button_arguments], button_action: true)
 
-            system_arguments[:button_arguments] = set_action_arguments(system_arguments[:button_arguments])
             add_option_to_mobile_menu(system_arguments[:button_arguments], mobile_icon, mobile_label, :default)
 
             Primer::OpenProject::PageHeader::Dialog.new(**system_arguments)
@@ -149,7 +160,7 @@ class PageHeader < Primer::Component
       #
       # @param items [Array<String, Hash>] Items is an array of strings, hash {href, text} or an anchor tag string
       # @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
-      renders_one :breadcrumbs, lambda { |items, **system_arguments|
+      renders_one :breadcrumbs, lambda { |items, selected_item_font_weight: :bold, **system_arguments|
         system_arguments[:classes] = class_names(system_arguments[:classes], "PageHeader-breadcrumbs")
         system_arguments[:display] ||= DEFAULT_BREADCRUMBS_DISPLAY
 
@@ -179,7 +190,7 @@ class PageHeader < Primer::Component
             item = anchor_string_to_object(item) if anchor_tag_string?(item)
 
             if item.is_a?(String)
-              breadcrumbs.with_item(href: "#") { item }
+              breadcrumbs.with_item(href: "#", font_weight: selected_item_font_weight) { item }
             else
               breadcrumbs.with_item(href: item[:href], target: "_top") { item[:text] }
             end
@@ -228,14 +239,18 @@ def render_mobile_menu?
 
       private
 
-      def set_action_arguments(system_arguments, scheme: nil)
+      def set_action_arguments(system_arguments, scheme: nil, button_action: false)
         system_arguments[:ml] ||= 2
         system_arguments[:display] = [:none, :flex]
         system_arguments[:scheme] = scheme unless scheme.nil?
         system_arguments[:classes] = class_names(
           system_arguments[:classes],
           "PageHeader-action",
         )
+        if button_action
+          system_arguments[:data] ||= {}
+          system_arguments[:data][:targets] = "page-header.actionItems"
+        end
 
         system_arguments[:id] ||= self.class.generate_id
         system_arguments

app/components/primer/open_project/page_header_element.ts

@@ -1,7 +1,22 @@
-import {controller} from '@github/catalyst'
+import {controller, targets} from '@github/catalyst'
 
 @controller
 class PageHeaderElement extends HTMLElement {
+  @targets actionItems: HTMLElement[]
+
+  connectedCallback() {
+    for (const item of this.actionItems) {
+      /*
+      If there is only one action to be shown, we show that instead of the mobile action menu. However, the buttons should be the smaller button variant.
+      Unfortunately, the `size` attribute does not support responsive attributes and the .pcss syntax does not support inheritance between classes.
+      So we have to add the class manually here.
+      */
+      if (window.innerWidth <= 544) {
+        item.classList.add('Button--small')
+      }
+    }
+  }
+
   menuItemClick(event: Event) {
     const currentTarget = event.currentTarget as HTMLButtonElement
 

previews/primer/open_project/page_header_preview.rb

@@ -137,19 +137,19 @@ def leading_action(href: "#", icon: :"arrow-left")
         end
       end
 
-      # @label With breadcrumbs
+      # @label With non-bold breadcrumbs
       # **Breadcrumbs** are only shown on **wider than narrow screens** by default.
       # A parent link is shown instead in narrow screens
-      #
-      def breadcrumbs
+      # Per default, the last element is shown in bold, but that can be disabled, e.g if only parts of the string should be bold.
+      def non_bold_breadcrumbs
         breadcrumb_items = [
           { href: "/foo", text: "Foo" },
           "\u003ca href=\"/foo/bar\"\u003eBar\u003c/a\u003e",
-          "Baz"
+          "Test: <b>Baz</b>".html_safe
         ]
         render(Primer::OpenProject::PageHeader.new) do |header|
           header.with_title { "A title" }
-          header.with_breadcrumbs(breadcrumb_items)
+          header.with_breadcrumbs(breadcrumb_items, selected_item_font_weight: :normal)
         end
       end
     end

test/components/primer/open_project/page_header_test.rb

@@ -189,7 +189,30 @@ def test_renders_breadcrumbs
 
     assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item a[href='/foo']")
     assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item a[href='/foo/bar']")
-    assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item a[href='#']")
+    assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item.text-bold a[href='#']")
+  end
+
+  def test_renders_non_bold_breadcrumbs
+    breadcrumb_items = [
+      { href: "/foo", text: "Foo" },
+      "\u003ca href=\"/foo/bar\"\u003eBar\u003c/a\u003e",
+      { href: "#", text: "test" },
+      "test"
+    ]
+
+    render_inline(Primer::OpenProject::PageHeader.new) do |header|
+      header.with_title { "Hello" }
+      header.with_breadcrumbs(breadcrumb_items, selected_item_font_weight: :normal)
+    end
+
+    assert_text("Hello")
+    assert_selector(".PageHeader-title")
+    assert_selector(".PageHeader-breadcrumbs")
+    assert_selector(".PageHeader-parentLink")
+
+    assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item a[href='/foo']")
+    assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item a[href='/foo/bar']")
+    assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item:not(.text-bold) a[href='#']")
   end
 
   private