Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reapply "Revert tab container upgrade" (#2855) #2909

Merged
merged 2 commits into from
Jun 21, 2024
Merged

Reapply "Revert tab container upgrade" (#2855) #2909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1039f31
Reapply "Revert tab container upgrade" (#2855)
camertron Jun 21, 2024
b131354
Add changeset
camertron Jun 21, 2024
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/perfect-jobs-pretend.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': patch
---

Revert tab-container-element upgrade
Binary file modified ...s/snapshots.test.ts-snapshots/primer/alpha/underline_panels/default/default.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...s/snapshots.test.ts-snapshots/primer/alpha/underline_panels/default/focused.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...pshots.test.ts-snapshots/primer/alpha/underline_panels/with_actions/default.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ...pshots.test.ts-snapshots/primer/alpha/underline_panels/with_actions/focused.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ....ts-snapshots/primer/alpha/underline_panels/with_icons_and_counters/default.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified ....ts-snapshots/primer/alpha/underline_panels/with_icons_and_counters/focused.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 0 additions & 10 deletions app/components/primer/alpha/tab_nav.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@
overflow: hidden;
}

.tabnav::part(tablist-wrapper) {
margin-bottom: var(--stack-gap-normal);
border-bottom: var(--borderWidth-thin) solid var(--borderColor-default);
}

.tabnav-tab {
display: inline-block;
flex-shrink: 0;
Expand Down Expand Up @@ -71,11 +66,6 @@
}
}

tab-container .tabnav-tab {
margin-bottom: -1px;
}


/* Tabnav extras
**
** Tabnav extras are non-tab elements that sit in the tabnav. Usually they're
Expand Down
14 changes: 9 additions & 5 deletions app/components/primer/alpha/tab_panels.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
<%= render Primer::BaseComponent.new(**@system_arguments) do %>
<%= extra if @align == :left %>
<% tabs.each do |tab| %>
<%= tab %>
<%= tab_container_wrapper(with_panel: true, **@wrapper_arguments) do %>
<%= render Primer::BaseComponent.new(**@system_arguments) do %>
<%= extra if @align == :left %>
<%= render Primer::BaseComponent.new(**@body_arguments) do %>
<% tabs.each do |tab| %>
<%= tab %>
<% end %>
<% end %>
<%= extra if @align == :right %>
<% end %>
<%= extra if @align == :right %>
<% tabs.each do |tab| %>
<%= tab.panel %>
<% end %>
Expand Down
17 changes: 13 additions & 4 deletions app/components/primer/alpha/tab_panels.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class TabPanels < Primer::Component
Primer::Alpha::Navigation::Tab.new(
selected: selected,
with_panel: true,
list: false,
list: true,
panel_id: "panel-#{id}",
**system_arguments
)
Expand All @@ -43,14 +43,23 @@ class TabPanels < Primer::Component

# @param label [String] Sets an `aria-label` that helps assistive technology users understand the purpose of the tabs.
# @param align [Symbol] <%= one_of(Primer::TabNavHelper::EXTRA_ALIGN_OPTIONS) %> - Defaults to <%= Primer::TabNavHelper::EXTRA_ALIGN_DEFAULT %>
# @param body_arguments [Hash] <%= link_to_system_arguments_docs %> for the body wrapper.
# @param wrapper_arguments [Hash] <%= link_to_system_arguments_docs %> for the `TabContainer` wrapper.
# @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
def initialize(label:, body_arguments: {}, wrapper_arguments: {}, **system_arguments)
@align = EXTRA_ALIGN_DEFAULT
@wrapper_arguments = wrapper_arguments

@system_arguments = { **deny_tag_argument(**system_arguments), **deny_tag_argument(**wrapper_arguments) }
@system_arguments[:tag] = :"tab-container"
@system_arguments = deny_tag_argument(**system_arguments)
@system_arguments[:tag] = :div
@system_arguments[:classes] = tab_nav_classes(@system_arguments[:classes])
@system_arguments[:"aria-label"] = label

@body_arguments = deny_tag_argument(**body_arguments)
@body_arguments[:tag] = :ul
@body_arguments[:classes] = tab_nav_body_classes(@body_arguments[:classes])

@body_arguments[:role] = :tablist
@body_arguments[:"aria-label"] = label
end

def before_render
Expand Down
8 changes: 1 addition & 7 deletions app/components/primer/alpha/underline_nav.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,7 @@
}
}

.UnderlineNav::part(tablist-wrapper) {
width: 100%;
box-shadow: inset 0 -1px 0 var(--borderColor-muted);
padding: var(--control-medium-gap) 0;
}

.UnderlineNav-body,.UnderlineNav::part(tablist) {
.UnderlineNav-body {
display: flex;
align-items: center;
gap: var(--control-medium-gap);
Expand Down
14 changes: 8 additions & 6 deletions app/components/primer/alpha/underline_panels.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
<%= render Primer::BaseComponent.new(**@wrapper_arguments) do %>
<%= tab_container_wrapper(with_panel: true, **@wrapper_arguments) do %>
<%= render Primer::BaseComponent.new(**@system_arguments) do %>
<% if @align == :right %>
<%= actions %>
<% end %>
<% tabs.each do |tab| %>
<%= tab %>
<%= render body do %>
<% tabs.each do |tab| %>
<%= tab %>
<% end %>
<% end %>
<% if @align == :left %>
<%= actions %>
<% end %>
<% tabs.each do |tab| %>
<%= tab.panel %>
<% end %>
<% end %>
<% tabs.each do |tab| %>
<%= tab.panel %>
<% end %>
<% end %>
4 changes: 0 additions & 4 deletions app/components/primer/alpha/underline_panels.pcss
View file
Open in desktop

This file was deleted.

20 changes: 14 additions & 6 deletions app/components/primer/alpha/underline_panels.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class UnderlinePanels < Primer::Component
Primer::Alpha::Navigation::Tab.new(
selected: selected,
with_panel: true,
list: false,
list: true,
icon_classes: "UnderlineNav-octicon",
panel_id: "panel-#{id}",
**system_arguments
Expand All @@ -43,16 +43,24 @@ class UnderlinePanels < Primer::Component
# @param system_arguments [Hash] <%= link_to_system_arguments_docs %>
def initialize(label:, align: ALIGN_DEFAULT, body_arguments: {}, wrapper_arguments: {}, **system_arguments)
@align = fetch_or_fallback(ALIGN_OPTIONS, align, ALIGN_DEFAULT)
@wrapper_arguments = deny_tag_argument(**wrapper_arguments)
@wrapper_arguments[:tag] = :div
@wrapper_arguments = wrapper_arguments

@system_arguments = deny_tag_argument(**system_arguments)
@system_arguments[:tag] = :"tab-container"
@system_arguments[:tag] = :div
@system_arguments[:classes] = underline_nav_classes(@system_arguments[:classes], @align)
@system_arguments[:"aria-label"] = label

@body_arguments = deny_tag_argument(**body_arguments)
@body_arguments[:tag] = :div
@body_arguments[:tag] = :ul
@body_arguments[:classes] = underline_nav_body_classes(@body_arguments[:classes])

@body_arguments[:role] = :tablist
@body_arguments[:"aria-label"] = label
end

private

def body
Primer::BaseComponent.new(**@body_arguments)
end
end
end
Expand Down
1 change: 0 additions & 1 deletion app/components/primer/primer.pcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
@import "./alpha/button_marketing.pcss";
@import "./alpha/toggle_switch.pcss";
@import "./alpha/underline_nav.pcss";
@import "./alpha/underline_panels.pcss";
@import "./alpha/segmented_control.pcss";
@import "./alpha/menu.pcss";

Expand Down
28 changes: 7 additions & 21 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"@github/image-crop-element": "^5.0.0",
"@github/include-fragment-element": "^6.1.1",
"@github/relative-time-element": "^4.0.0",
"@github/tab-container-element": "^4.5.0",
"@github/tab-container-element": "^3.1.2",
"@oddbird/popover-polyfill": "^0.4.0",
"@primer/behaviors": "^1.3.4"
},
Expand Down
24 changes: 20 additions & 4 deletions test/components/alpha/tab_panels_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,16 @@ def test_renders_tabs_and_panels_with_relevant_aria_attributes
end

assert_selector("tab-container") do
assert_selector("button#tab-1.tabnav-tab[aria-selected='true']", text: "Tab 1")
assert_selector("button#tab-2.tabnav-tab", text: "Tab 2")
assert_selector("div.tabnav") do
assert_selector("ul.tabnav-tabs[aria-label='label']") do
assert_selector("li") do
assert_selector("button#tab-1.tabnav-tab[aria-selected='true']", text: "Tab 1")
end
assert_selector("li") do
assert_selector("button#tab-2.tabnav-tab", text: "Tab 2")
end
end
end
assert_selector("div#panel-tab-1[aria-labelledby='tab-1']", text: "Panel 1")
assert_selector("div#panel-tab-2[aria-labelledby='tab-2']", text: "Panel 2", visible: false)
end
Expand Down Expand Up @@ -62,8 +70,16 @@ def test_renders_extra_content
component.with_extra { "extra" }
end
assert_selector("tab-container") do
assert_selector("button#tab-1.tabnav-tab[aria-selected='true']", text: "Tab 1")
assert_selector("button#tab-2.tabnav-tab", text: "Tab 2")
assert_selector("div.tabnav") do
assert_selector("ul.tabnav-tabs[aria-label='label']") do
assert_selector("li") do
assert_selector("button#tab-1.tabnav-tab[aria-selected='true']", text: "Tab 1")
end
assert_selector("li") do
assert_selector("button#tab-2.tabnav-tab", text: "Tab 2")
end
end
end
assert_selector("div#panel-tab-1[aria-labelledby='tab-1']", text: "Panel 1")
assert_selector("div#panel-tab-2[aria-labelledby='tab-2']", text: "Panel 2", visible: false)
assert_text("extra")
Expand Down
48 changes: 35 additions & 13 deletions test/components/alpha/underline_panels_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,21 @@ def test_renders_panels_and_tab_container
end
end

assert_selector("tab-container.UnderlineNav") do
assert_selector("button.UnderlineNav-item[role='tab'][aria-selected='true']", text: "Tab 1")
assert_selector("button.UnderlineNav-item[role='tab']", text: "Tab 2")
assert_selector("div.UnderlineNav-actions", text: "Actions content")
assert_selector("tab-container") do
assert_selector("div.UnderlineNav") do
assert_selector("ul.UnderlineNav-body[role='tablist'][aria-label='label']") do
assert_selector("li[role='presentation']") do
assert_selector("button.UnderlineNav-item[role='tab'][aria-selected='true']", text: "Tab 1")
end
assert_selector("li[role='presentation']") do
assert_selector("button.UnderlineNav-item[role='tab']", text: "Tab 2")
end
end
assert_selector("div.UnderlineNav-actions", text: "Actions content")
end
assert_selector("div[role='tabpanel']", text: "Panel 1")
assert_selector("div[role='tabpanel']", text: "Panel 2", visible: false)
end
assert_selector("div[role='tabpanel']", text: "Panel 1")
assert_selector("div[role='tabpanel']", text: "Panel 2", visible: false)
end

def test_customizes_tab_container
Expand All @@ -37,7 +45,7 @@ def test_customizes_tab_container
end
end

assert_selector("div.m-2.custom-class")
assert_selector("tab-container.m-2.custom-class")
end

def test_raises_if_multiple_tabs_are_selected
Expand Down Expand Up @@ -101,9 +109,15 @@ def test_align_falls_back_to_default

refute_selector(".UnderlineNav--right")

assert_selector("tab-container.UnderlineNav") do
assert_selector("button.UnderlineNav-item[role='tab'][aria-selected='true']", text: "Tab 1")
assert_selector("button.UnderlineNav-item[role='tab']", text: "Tab 2")
assert_selector("div.UnderlineNav") do
assert_selector("ul.UnderlineNav-body[aria-label='label']") do
assert_selector("li[role='presentation']") do
assert_selector("button.UnderlineNav-item[role='tab'][aria-selected='true']", text: "Tab 1")
end
assert_selector("li[role='presentation']") do
assert_selector("button.UnderlineNav-item[role='tab']", text: "Tab 2")
end
end
assert_selector("div.UnderlineNav-actions", text: "Actions content")
end
end
Expand All @@ -123,9 +137,15 @@ def test_adds_underline_nav_right_when_align_right_is_set
end
end

assert_selector("tab-container.UnderlineNav.UnderlineNav--right") do
assert_selector("button.UnderlineNav-item[role='tab'][aria-selected='true']", text: "Tab 1")
assert_selector("button.UnderlineNav-item[role='tab']", text: "Tab 2")
assert_selector("div.UnderlineNav.UnderlineNav--right") do
assert_selector("ul.UnderlineNav-body[aria-label='label']") do
assert_selector("li[role='presentation']") do
assert_selector("button.UnderlineNav-item[role='tab'][aria-selected='true']", text: "Tab 1")
end
assert_selector("li[role='presentation']") do
assert_selector("button.UnderlineNav-item[role='tab']", text: "Tab 2")
end
end
assert_selector("div.UnderlineNav-actions", text: "Actions content")
end
end
Expand All @@ -144,6 +164,8 @@ def test_puts_actions_first_if_align_right_and_actions_exist
"Actions content"
end
end

assert_selector(".UnderlineNav > .UnderlineNav-body:last-child")
end

def test_renders_tab_icon_with_correct_classes
Expand Down
1 change: 0 additions & 1 deletion test/css/component_specific_selectors_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ class ComponentSpecificSelectorsTest < Minitest::Test
],
Primer::Alpha::TabNav => [
".tabnav-tab.selected",
"tab-container .tabnav-tab",
".tabnav-extra",
".tabnav-btn"
],
Expand Down
Loading
Loading