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.

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

[SelectPanel] Improve messages API #3146

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/forty-cooks-drive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/view-components': minor
---

Improve SelectPanel messages API (no items/no results, errors)
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
44 changes: 23 additions & 21 deletions app/components/primer/alpha/select_panel.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,10 @@
<% end %>
<% if show_filter? %>
<% header.with_filter do %>
<div data-target="select-panel.bannerErrorElement" hidden>
<%= render Primer::Alpha::Banner.new(scheme: @banner_scheme, mb: 2) do %>
<% if error_content? %>
<%= error_content %>
<% else %>
<h2 class="f6 text-normal">Sorry, something went wrong.</h2>
<% end %>
<% end %>
</div>
<% unless banner_error_message? %>
<% with_banner_error_message { "Sorry, something went wrong." } %>
<% end %>
<%= banner_error_message %>
<%= render(Primer::BaseComponent.new(
tag: :"remote-input",
aria: { owns: @body_id },
Expand Down Expand Up @@ -76,25 +71,32 @@
<div id="<%= @loading_description_id %>" aria-hidden="true"><%= @loading_description %></div>
<% end %>
</div>
<div data-show-on-error hidden data-target="select-panel.fragmentErrorElement">
<% if preload_error_content? %>
<%= preload_error_content %>
<% else %>
<div class="pt-2 pb-2">
<%= render Primer::Beta::Octicon.new(icon: :alert, color: :danger) %>
<h2 class="f5 mt-2">Sorry, something went wrong.</h2>
</div>
<% unless body_error_message? %>
<% with_body_error_message do |message| %>
<% message.with_title { "Sorry, something went wrong." } %>
<% message.with_description { "Try again or if the problem persists, contact support." } %>
<% end %>
</div>
<% end %>
<%= body_error_message %>
<% end %>
<% end %>
<% else %>
<%= render(@list) %>
<% end %>
</div>
<div data-target="select-panel.noResults" class="pt-2 color-border-muted text-center d-flex flex-items-start flex-justify-center SelectPanel-emptyPanel" hidden>
<h2 class="v-align-middle m-3 f5"><%= @no_results_label %></h2>
</div>
<% unless no_items_message? %>
<% with_no_items_message do |message| %>
<% message.with_title { "No items found." } %>
<% end %>
<% end %>
<%= no_items_message %>
<% unless no_matches_message? %>
<% with_no_matches_message do |message| %>
<% message.with_title { "No matches found." } %>
<% message.with_description { "Try adjusting your search terms." } %>
<% end %>
<% end %>
<%= no_matches_message %>
<% end %>
</focus-group>
<% end %>
Expand Down
45 changes: 38 additions & 7 deletions app/components/primer/alpha/select_panel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,31 @@ def with_avatar_item(**system_arguments)
end
end

# The message rendered in place of the list of items if an error occurrs when fetching items for the first time.
class BodyErrorMessage < Primer::Component
renders_one :title
renders_one :description
end

# The message rendered in place of the list of items if there are no items and no filter text.
class NoItemsMessage < Primer::Component
renders_one :title
renders_one :description
end

# The message rendered in place of the list of items if there are no items that match the filter text.
class NoMatchesMessage < Primer::Component
renders_one :title
renders_one :description
end

# The message rendered above the filter input when an error occurs when fetching items as the result of a filter operation.
class BannerErrorMessage < Primer::Component
def initialize(scheme: DEFAULT_BANNER_SCHEME)
@scheme = scheme
end
end

status :alpha

DEFAULT_PRELOAD = false
Expand Down Expand Up @@ -368,7 +393,6 @@ def with_avatar_item(**system_arguments)
# @param size [Symbol] The size of the panel. <%= one_of(Primer::Alpha::Overlay::SIZE_OPTIONS) %>
# @param select_variant [Symbol] <%= one_of(Primer::Alpha::SelectPanel::SELECT_VARIANT_OPTIONS) %>
# @param fetch_strategy [Symbol] <%= one_of(Primer::Alpha::SelectPanel::FETCH_STRATEGIES) %>
# @param no_results_label [String] The label to display when no results are found.
# @param preload [Boolean] Whether to preload search results when the page loads. If this option is false, results are loaded when the panel is opened.
# @param dynamic_label [Boolean] Whether or not to display the text of the currently selected item in the show button.
# @param dynamic_label_prefix [String] If provided, the prefix is prepended to the dynamic label and displayed in the show button.
Expand All @@ -391,7 +415,6 @@ def initialize(
size: :small,
select_variant: DEFAULT_SELECT_VARIANT,
fetch_strategy: DEFAULT_FETCH_STRATEGY,
no_results_label: "No results found",
preload: DEFAULT_PRELOAD,
dynamic_label: false,
dynamic_label_prefix: nil,
Expand All @@ -405,7 +428,7 @@ def initialize(
anchor_side: Primer::Alpha::Overlay::DEFAULT_ANCHOR_SIDE,
loading_label: "Loading content...",
loading_description: nil,
banner_scheme: DEFAULT_BANNER_SCHEME,
banner_scheme: nil,
**system_arguments
)
raise_if_role_given!(**system_arguments)
Expand All @@ -422,7 +445,6 @@ def initialize(
@preload = fetch_or_fallback_boolean(preload, DEFAULT_PRELOAD)
@select_variant = fetch_or_fallback(SELECT_VARIANT_OPTIONS, select_variant, DEFAULT_SELECT_VARIANT)
@fetch_strategy = fetch_or_fallback(FETCH_STRATEGIES, fetch_strategy, DEFAULT_FETCH_STRATEGY)
@no_results_label = no_results_label
@show_filter = show_filter
@dynamic_label = dynamic_label
@dynamic_label_prefix = dynamic_label_prefix
Expand All @@ -433,7 +455,7 @@ def initialize(
@loading_description_id = "#{@panel_id}-loading-description"
end
@loading_description = loading_description
@banner_scheme = fetch_or_fallback(BANNER_SCHEME_OPTIONS, banner_scheme, DEFAULT_BANNER_SCHEME)
@banner_scheme = banner_scheme

@system_arguments = deny_tag_argument(**system_arguments)
@system_arguments[:id] = @panel_id
Expand Down Expand Up @@ -534,12 +556,21 @@ def initialize(
# Customizable content for the error message that appears when items are fetched for the first time. This message
# appears in place of the list of items.
# For more information, see the [documentation regarding SelectPanel error messaging](/components/selectpanel#errorwarning).
renders_one :preload_error_content
renders_one :body_error_message, BodyErrorMessage

# Customizable content for the error message that appears when items are fetched as the result of a filter
# operation. This message appears as a banner above the previously fetched list of items.
# For more information, see the [documentation regarding SelectPanel error messaging](/components/selectpanel#errorwarning).
renders_one :error_content
renders_one :banner_error_message, -> (scheme: nil) {
scheme ||= @banner_scheme || DEFAULT_BANNER_SCHEME

BannerErrorMessage.new(
scheme: fetch_or_fallback(BANNER_SCHEME_OPTIONS, scheme, DEFAULT_BANNER_SCHEME)
)
}

renders_one :no_items_message, NoItemsMessage
renders_one :no_matches_message, NoMatchesMessage

private

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<div data-target="select-panel.bannerErrorMessage" hidden>
<%= render Primer::Alpha::Banner.new(scheme: @scheme, mb: 2) do %>
<h2 class="f6 text-normal"><%= content %></h2>
<% end %>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<div data-show-on-error hidden data-target="select-panel.bodyErrorMessage" class="p-3">
<%= render Primer::Beta::Octicon.new(icon: :alert, color: :danger) %>
<h2 class="f5 mt-2">
<%= title %>
</h2>
<% if description? %>
<h3 class="f6 text-normal color-fg-muted mt-1">
<%= description %>
</h3>
<% end %>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<div data-target="select-panel.noItemsMessage" class="p-3 d-flex flex-items-center flex-column flex-justify-start SelectPanel-emptyPanel" hidden>
<h2 class="f5">
<%= title %>
</h2>
<% if description? %>
<h3 class="f6 text-normal color-fg-muted mt-1">
<%= description %>
</h3>
<% end %>
</div>
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<div data-target="select-panel.noMatchesMessage" class="p-3 d-flex flex-items-center flex-column flex-justify-start SelectPanel-emptyPanel" hidden>
<h2 class="f5">
<%= title %>
</h2>
<% if description? %>
<h3 class="f6 text-normal color-fg-muted mt-1">
<%= description %>
</h3>
<% end %>
</div>
71 changes: 46 additions & 25 deletions app/components/primer/alpha/select_panel_element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@ export class SelectPanelElement extends HTMLElement {
@target remoteInput: HTMLElement
@target list: HTMLElement
@target ariaLiveContainer: HTMLElement
@target noResults: HTMLElement
@target fragmentErrorElement: HTMLElement
@target bannerErrorElement: HTMLElement
@target noItemsMessage: HTMLElement
@target noMatchesMessage: HTMLElement
@target bodyErrorMessage: HTMLElement
@target bannerErrorMessage: HTMLElement
@target bodySpinner: HTMLElement

filterFn?: FilterFn
Expand Down Expand Up @@ -558,7 +559,7 @@ export class SelectPanelElement extends HTMLElement {
case 'error': {
this.#toggleIncludeFragmentElements(true)

const errorElement = this.fragmentErrorElement
const errorElement = this.bodyErrorMessage
// check if the errorElement is visible in the dom
if (errorElement && !errorElement.hasAttribute('hidden')) {
announceFromElement(errorElement, {element: this.ariaLiveContainer, assertive: true})
Expand Down Expand Up @@ -686,9 +687,9 @@ export class SelectPanelElement extends HTMLElement {
if (!this.list) return

let atLeastOneResult = false
const query = this.filterInputTextField?.value ?? ''

if (this.#performFilteringLocally()) {
const query = this.filterInputTextField?.value ?? ''
const filter = this.filterFn || this.#defaultFilterFn

for (const item of this.items) {
Expand All @@ -704,7 +705,6 @@ export class SelectPanelElement extends HTMLElement {
}

this.#updateTabIndices()
this.#maybeAnnounce()

for (const item of this.items) {
const itemContent = this.#getItemContent(item)
Expand All @@ -722,43 +722,57 @@ export class SelectPanelElement extends HTMLElement {

this.#hasLoadedData = true

if (!this.noResults) return

if (this.#inErrorState()) {
this.noResults.setAttribute('hidden', '')
this.noMatchesMessage?.setAttribute('hidden', '')
this.noItemsMessage?.setAttribute('hidden', '')
return
}

if (atLeastOneResult) {
this.noResults.setAttribute('hidden', '')
// TODO can we change this to search for `@panelId-list`
this.noMatchesMessage?.setAttribute('hidden', '')
this.noItemsMessage?.setAttribute('hidden', '')

// TODO can we change this to search for `@panelId-list`?
this.list?.querySelector('.ActionListWrap')?.removeAttribute('hidden')
} else {
} else if (!this.#isLoading()) {
if (query === '') {
this.noMatchesMessage?.setAttribute('hidden', '')
this.noItemsMessage?.removeAttribute('hidden')
} else {
this.noMatchesMessage?.removeAttribute('hidden')
this.noItemsMessage?.setAttribute('hidden', '')
}

this.list?.querySelector('.ActionListWrap')?.setAttribute('hidden', '')
this.noResults.removeAttribute('hidden')
}

this.#maybeAnnounce()
}

#isLoading(): boolean {
return Boolean(this.bodySpinner) || (this.#filterInputTextFieldElement?.isLoading() ?? false)
}

#inErrorState(): boolean {
if (this.fragmentErrorElement && !this.fragmentErrorElement.hasAttribute('hidden')) {
if (this.bodyErrorMessage && !this.bodyErrorMessage.hasAttribute('hidden')) {
return true
}

if (!this.bannerErrorElement) return false
if (!this.bannerErrorMessage) return false

return !this.bannerErrorElement.hasAttribute('hidden')
return !this.bannerErrorMessage.hasAttribute('hidden')
}

#setErrorState(type: ErrorStateType) {
let errorElement = this.fragmentErrorElement
let errorElement = this.bodyErrorMessage

if (type === ErrorStateType.BODY) {
this.fragmentErrorElement?.removeAttribute('hidden')
this.bannerErrorElement.setAttribute('hidden', '')
this.bodyErrorMessage?.removeAttribute('hidden')
this.bannerErrorMessage.setAttribute('hidden', '')
} else {
errorElement = this.bannerErrorElement
this.bannerErrorElement?.removeAttribute('hidden')
this.fragmentErrorElement?.setAttribute('hidden', '')
errorElement = this.bannerErrorMessage
this.bannerErrorMessage?.removeAttribute('hidden')
this.bodyErrorMessage?.setAttribute('hidden', '')
}

// check if the errorElement is visible in the dom
Expand All @@ -769,8 +783,8 @@ export class SelectPanelElement extends HTMLElement {
}

#clearErrorState() {
this.fragmentErrorElement?.setAttribute('hidden', '')
this.bannerErrorElement.setAttribute('hidden', '')
this.bodyErrorMessage?.setAttribute('hidden', '')
this.bannerErrorMessage.setAttribute('hidden', '')
}

#maybeAnnounce() {
Expand All @@ -783,7 +797,14 @@ export class SelectPanelElement extends HTMLElement {
element: this.ariaLiveContainer,
})
} else {
const noResultsEl = this.noResults
const noResultsEl = (() => {
if (this.noMatchesMessage && !this.noMatchesMessage.hasAttribute('hidden')) {
return this.noMatchesMessage
}

return this.noItemsMessage
})()

if (noResultsEl) {
announceFromElement(noResultsEl, {element: this.ariaLiveContainer})
}
Expand Down
4 changes: 4 additions & 0 deletions app/lib/primer/forms/primer_text_field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,8 @@ export class PrimerTextFieldElement extends HTMLElement {
this.leadingSpinner?.setAttribute('hidden', '')
this.leadingVisual?.removeAttribute('hidden')
}

isLoading(): boolean {
return !this.leadingSpinner?.hasAttribute('hidden')
}
}
2 changes: 1 addition & 1 deletion previews/primer/alpha/select_panel_preview.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ def playground(
render_with_template(locals: {
subtitle: subtitle,
selected_items: selected_items,
no_results_label: no_results_label,
system_arguments: {
title: title,
size: size,
simulate_failure: simulate_failure,
simulate_no_results: simulate_no_results,
no_results_label: no_results_label,
dynamic_label: dynamic_label,
dynamic_label_prefix: dynamic_label_prefix,
dynamic_aria_label_prefix: dynamic_aria_label_prefix,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@
<% if subtitle %>
<% panel.with_subtitle { subtitle } %>
<% end %>
<% if no_results_label %>
<% panel.with_no_items_message do |message| %>
<% message.with_title { no_results_label } %>
<% end %>
<% panel.with_no_matches_message do |message| %>
<% message.with_title { no_results_label } %>
<% end %>
<% end %>
<% end %>

<%= render partial: "primer/alpha/select_panel_preview/interaction_subject_js", locals: { subject_id: subject_id } %>
Loading
Loading