flop_phoenix
flop_phoenix copied to clipboard
Add No Results Row
Description
When a Flop table has no rows in the table body, there is no messaging about empty contents. Although, :no_results_content
is listed as an option, it appears to have no impact on the UI.
This PR adds a :no_results_attrs
option and a new <tr>
that is rendered when there are no @items
. This solution works with both lists and LiveStreams.
Checklist
- [ ] I added tests that cover my proposed changes.
- [ ] I updated the documentation related to my changes (if applicable).
@woylie This is a follow up from the previous discussion. I was reminded that CSS pseudo selectors can be used for this type of scenario, where we can't check a LiveStream for emptiness.
Hat tip to @clarkware. 🤠
Thanks for opening this PR. I have a few thoughts to consider.
- This library generally adheres to using semantic class names, and
only:table-row
is a Tailwind class. - From a semantic perspective, a paragraph may be a more fitting choice than a table with headers and a single cell in the body.
- Using a paragraph would likely offer a better experience for users who depend on screen readers.
If we decide to stick with the original approach instead, encapsulating the condition in a named function could improve clarity:
defp no_results?([]), do: true
defp no_results?(%LiveStream{inserts: []}), do: true
defp no_results?(_), do: false
However, the LiveStream struct is undocumented. I'm also not sure whether it is sufficient to just look at the inserts
field.
An alternative could be adding an empty?
attribute to the table component that defaults to false
. The condition used in the component would then change to if @items == [] || @empty?
. This allows the empty flag to be set on the socket based on query results, should you choose to use a stream.
Thank you for the feedback, and sorry I didn't see your response earlier. There's a bit more complexity here than I expected, so please forgive the lengthy response below.
CSS Classes
I could set a style
attr instead, but that would be disallowed when CSP is applied - unless the developer configures CSP correctly to allow inline styles. I ran into this issue when I enabled CSP on a new Phoenix project, where root.html.heex
had a style attr on one of the elements iirc. Using a semantic name would follow the pattern you've set, but it would require each developer to know how to apply display
properly if the row is the only child.
Conditional Rendering
My goal was to make this something the developer doesn't need to think about. I think conditionally rendering a <table>
or <p>
based on @items
emptiness would be fine, but I'm also not sure how reliable LiveStream.inserts
is for detecting emptiness. Adding an empty?
attr wouldn't add much value in my opinion, since it would still require each developer to know how to detect emptiness, which is something they could already do.
While it's possible I had a wrong implementation, I believe I wasn't seeing the "No results" row when I used a similar approach that checked @items
for an empty list or a stream with empty inserts.
Existing Patterns
Rendering a <p>
instead of a <table>
when there is no data may make sense, but the "No results" <tr>
seems to be a well-established pattern. Searching "table with no data" on Google, the examples I see almost always render a table with a row conveying that there is no data. Maybe aria attributes could be used to clarify things for screen readers.
It's not obvious what to do next. I completely understand wanting to keep the components CSS framework-agnostic, but we'd need to figure out where to put custom styles if this is going to be something devs don't need to think about.
Using a semantic name would follow the pattern you've set, but it would require each developer to know how to apply
display
properly if the row is the only child.
That's true, but the same is true for the Tailwind class if your project does not use Tailwind. It would be odd to have a class that's specific for one single framework. If we were to add this, I'd want to use a generic class name like is-only-child-visible
or has-no-siblings-visible
to be consistent. The necessary CSS could be added to the documentation, including a Tailwind example. This should probably be opt-in to prevent a breaking change.
Adding an
empty?
attr wouldn't add much value in my opinion, since it would still require each developer to know how to detect emptiness, which is something they could already do.
It depends on the situation, but if it's a paginated table where you replace the stream whenever you paginate, then you know whether there are results or not when you fetch the results. In that case, you could do something like:
def handle_params(params, _, socket) do
{items, meta} = list_items(params)
{:noreply, socket |> assign(:empty?, items == []) |> stream(items, replace: true)}
end
This doesn't work in an infinite scroll or more-button scenario where you keep adding items without removing the old ones, of course.
Maybe aria attributes could be used to clarify things for screen readers.
I don't think we can do anything with aria attributes about this. Here are some examples about how screen readers announce tables: https://a11y-101.com/development/tables. Screen readers would always see and announce the table. We cannot conditionally add aria attributes to part of the table, since it's a CSS based solution.
Side note, I just found that WCAG suggests to use role="status"
for these kind of status messages (source). We should add that to the existing no-results message.