Skip to content
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
2fdaf97
Change page header component and make the breadcumbs optional, show c…
bsatarnejad Oct 24, 2025
7c51730
Create thin-terms-shout.md
bsatarnejad Oct 24, 2025
4e4febc
Generating component snapshots
bsatarnejad Oct 24, 2025
ab5ba5e
update page header test
bsatarnejad Oct 24, 2025
c81040e
Generating component snapshots
bsatarnejad Oct 24, 2025
1d11e23
add a test for rendering page header without breadcrumbs
bsatarnejad Oct 24, 2025
1b87f77
add a test to check if the context bar exist only for mobile actions
bsatarnejad Oct 24, 2025
c6fad7f
when there is no breadcrumbs, show actions in the title bar
bsatarnejad Oct 24, 2025
4134e0f
simplify rendering mobile actions
bsatarnejad Oct 24, 2025
76a9f09
show contextbar in mobile mode
bsatarnejad Oct 27, 2025
8dcc937
make the breadcrumbs required
bsatarnejad Oct 27, 2025
33b6cf8
move breadcrumbs previews close to each other
bsatarnejad Oct 27, 2025
13a6aa5
change test
bsatarnejad Oct 27, 2025
b6e95c2
move show_state out of private section
bsatarnejad Oct 27, 2025
f400cf5
add class for no breadcrumbs state before rendering the component
bsatarnejad Oct 28, 2025
e454ec4
add class for no breadcrumbs state while rendering the component
bsatarnejad Oct 28, 2025
940061a
add class for no breadcrumbs state in rendering breadcrumbs
bsatarnejad Oct 29, 2025
d519a8b
handle showing and hiding context bar in page ehader
bsatarnejad Oct 29, 2025
37d32c3
check for added class in the test
bsatarnejad Oct 29, 2025
382be26
change the patch to minor in the changeset
bsatarnejad Oct 29, 2025
9d76d51
remove default nil value for breadcrumbs
bsatarnejad Oct 30, 2025
1641c66
fix style lint errors
bsatarnejad Oct 30, 2025
a8ddab6
add comment to the styles
bsatarnejad Nov 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/thin-terms-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@openproject/primer-view-components": minor
---

[67724] keep breadcrumbs slot required but allow nil/empty value
15 changes: 15 additions & 0 deletions app/components/primer/open_project/page_header.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,18 @@
.PageHeader-parentLink {
flex: 1 1 auto;
}

.PageHeader--noBreadcrumb .PageHeader-contextBar {
display: none;
}

/* Keep it visible on smaller */
@media (max-width: 543.98px) {
.PageHeader--noBreadcrumb .PageHeader-contextBar {
display: flex;
}
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.PageHeader--noBreadcrumb .PageHeader-contextBar {
display: none;
}
/* Keep it visible on smaller */
@media (max-width: 543.98px) {
.PageHeader--noBreadcrumb .PageHeader-contextBar {
display: flex;
}
}
/* Hide the context bar on desktop when no breadcrumb is visible */
@media screen and (min-width: $breakpoint-sm) {
.PageHeader--noBreadcrumb .PageHeader-contextBar {
display: none;
}
}


.PageHeader--noBreadcrumb .PageHeader-titleBar {
height: var(--control-small-size);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was probably done for the alignment with the menu button? Could you please add a comment for that? Otherwise, it is weird to understand later.

}
12 changes: 8 additions & 4 deletions app/components/primer/open_project/page_header.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class PageHeader < Primer::Component
"PageHeader-title",
"PageHeader-title--#{variant}"
)

Primer::OpenProject::PageHeader::Title.new(state: @state, **system_arguments)
}

Expand Down Expand Up @@ -181,12 +180,17 @@ class PageHeader < Primer::Component
Primer::Beta::IconButton.new(icon: icon, **system_arguments)
}

# Optional breadcrumbs above the title row
# By default shown on wider screens. Can be overridden with system_argument: display
#
# Using PageHeader without breadcrumbs is only recommended in special cases.
# In doubt, please check the PageHeader component documentation.
# @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, selected_item_font_weight: :bold, **system_arguments|
if items.nil?
# No breadcrumbs → mark the PageHeader with a special class
@system_arguments[:classes] = class_names(@system_arguments[:classes], "PageHeader--noBreadcrumb")
return
end

system_arguments[:classes] = class_names(system_arguments[:classes], "PageHeader-breadcrumbs")
system_arguments[:display] ||= DEFAULT_BREADCRUMBS_DISPLAY

Expand Down
36 changes: 26 additions & 10 deletions previews/primer/open_project/page_header_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,32 @@ def non_bold_breadcrumbs
end
end

# @label Without breadcrumbs
# A PageHeader example that renders without breadcrumbs.
Comment thread
bsatarnejad marked this conversation as resolved.
# This should only be done for *entry pages* (such as overview pages).
# In case of doubt, please check with the Dream team.
Comment thread
HDinger marked this conversation as resolved.
def without_breadcrumbs
render(Primer::OpenProject::PageHeader.new) do |header|
header.with_title { "Hello" }
header.with_breadcrumbs(nil)
header.with_description { "This PageHeader does not have any breadcrumbs." }
header.with_action_button(mobile_icon: "star", mobile_label: "Star") do |button|
button.with_leading_visual_icon(icon: "star")
"Star"
end
end
end

# @label With skipable breadcrumb items
def skip_breadcrumb_item
render(Primer::OpenProject::PageHeader.new) do |component|
component.with_title { "Resize me to mobile screen size" }
component.with_breadcrumbs([{ href: "/foo", text: "Foo" },
{ href: "/bar", text: "Bar", skip_for_mobile: true },
"Baz"])
end
end

# @label With tab nav
#
def tab_nav
Expand Down Expand Up @@ -257,16 +283,6 @@ def segmented_control_mobile_icons
end
end

# @label With skipable breadcrumb items
def skip_breadcrumb_item
render(Primer::OpenProject::PageHeader.new) do |component|
component.with_title { "Resize me to mobile screen size" }
component.with_breadcrumbs([{ href: "/foo", text: "Foo" },
{ href: "/bar", text: "Bar", skip_for_mobile: true },
"Baz"])
end
end

# @label With a link in the description
def description
render_with_template(template: "primer/open_project/page_header_preview/description")
Expand Down
18 changes: 18 additions & 0 deletions test/components/primer/open_project/page_header_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,24 @@ def test_skips_a_breadcrumb_item_for_mobile
assert_selector("nav[aria-label='Breadcrumb'].PageHeader-breadcrumbs .breadcrumb-item.text-bold a[href='#']")
end

def test_renders_without_breadcrumbs
render_inline(Primer::OpenProject::PageHeader.new) do |header|
header.with_title { "Hello" }
header.with_breadcrumbs(nil)
end

# Title is rendered
assert_text("Hello")
assert_selector(".PageHeader-title")

# Breadcrumbs and parent link are not rendered
assert_no_selector(".PageHeader-breadcrumbs")
Comment thread
bsatarnejad marked this conversation as resolved.
assert_no_selector(".PageHeader-parentLink")

# A class should be added to the page header when there is no breadcrumbs
assert_selector(".PageHeader--noBreadcrumb")
end

private

def breadcrumb_elements
Expand Down
Loading