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

Circulars Archive Table Format #1816

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

tylerbarna
Copy link
Member

This is an experiment with changing the circulars archive to using the sortable table component found in react-uswds. As of the initial submission of this draft PR, the sorting aspect of the table is not functional. Additionally, I was hoping to add a column with the submission date for each circular, but this will require adding the createdOn metadata to the CircularMetadata interface since its currently only present on the Circular interface

tylerbarna and others added 5 commits December 20, 2023 16:07
…is present, but the sortiing aspect is not functional. Will have to investigate further. Additionally, it would be useful to make other metadata items, namely the published time, available
switching to the Circular interface now passes along the createdOn attribute so it can be shown as part of the results
the table format can now be enabled or disabled by including CIRCULARS_TABLE_FORMAT as a feature flag in ones .env file. If not provided, defaults to the existing list.
table is no longer horizontally scrollable but will rather take up the full width of the page
@tylerbarna
Copy link
Member Author

tylerbarna commented Jan 15, 2024

this is how it currently looks
image

The sorting aspect does not work at the moment

@lpsinger
Copy link
Member

@tylerbarna, this is marked as a draft. When you are ready for feedback, please remember to remove the draft status.

@tylerbarna
Copy link
Member Author

@tylerbarna, this is marked as a draft. When you are ready for feedback, please remember to remove the draft status.

ah, thanks, was waiting until I got the createdOn attribute working, but I forgot to mark it as ready for review after that

@tylerbarna tylerbarna marked this pull request as ready for review January 16, 2024 16:49
@lpsinger
Copy link
Member

lpsinger commented Jan 16, 2024

Hmm, this triples the amount of vertical space taken up by each item.

@tylerbarna
Copy link
Member Author

Hmm, this triples the amount of vertical space taken up by each item.

yeah, I noticed it's a bit less compact than I would like; I think that's partly because of the overflow caused by the submission time, which takes up two lines. I think some additional spacing between circulars would improve parsability of the page, but 3x the vertical space is a bit too much

@tylerbarna
Copy link
Member Author

@lpsinger how's this?
image

This is dependent on the length of the subjects on a given page, however
image

22 results vs 14 results in the same vertical space

@lpsinger
Copy link
Member

That's better. What did you change?

@tylerbarna
Copy link
Member Author

That's better. What did you change?

I changed it to usa-table--compact

@tylerbarna
Copy link
Member Author

may need to tweak it so it's the correct fixed width to match the rest of the content on the page; I'm noticing it's a bit short on the right side in comparison to the standard usa-table, which fills the horizontal space

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

  • This should not have a feature flag.
  • The CircularMetadata type represents the return value of the backend search function, and should contain all of the information that is displayed on the search results index. Please use that type; update it with the fields that you are adding to the search results.

@tylerbarna
Copy link
Member Author

Addressed both of the above, also set it so the table is always the same width as the rest of the content on the page for consistency

@tylerbarna
Copy link
Member Author

tylerbarna commented Jan 16, 2024

would it be beneficial to format the creation time in a different way, or is ISOT sufficient?

@lpsinger
Copy link
Member

I am ambivalent on this visual change. @jracusin, @dakota002, @Courey, what do you think?

@dakota002
Copy link
Contributor

I like this table display. A couple of small changes, not super important, but might help:

  • the "Created On" could probably be better represented with the <TimeAgo> component
  • Left align the header texts, instead of centered (Circular ID, Subject, Created On)

@tylerbarna can you expand on your comment of the UI being dependent on the length of the subjects? I think of your two screenshots from a couple days ago, the first one looks better, the header text shouldn't split onto two lines

@tylerbarna
Copy link
Member Author

  • Left align the header texts, instead of centered (Circular ID, Subject, Created On)

@dakota002 I agree with the left alignment on the headers, just pushed that.

  • the "Created On" could probably be better represented with the <TimeAgo> component

I tried the component, but I think switching to that somewhat reduces the utility of having the submitted times since all the items on a page end up being the same if they're less recent
image
I'd maybe prefer an inverse, where we show the date and time in the table, but the relative time in the past can be shown by mousing over the createdOn value

@tylerbarna can you expand on your comment of the UI being dependent on the length of the subjects? I think of your two screenshots from a couple days ago, the first one looks better, the header text shouldn't split onto two lines

If one of the subjects in that set of 100 circulars has a certain number of characters, it will result in the subject overflowing onto a second line, and the entire table will be double height for each cell to maintain visual consistency. If we don't want the height of the table to change depending on the subject length, we could clip the circular subjects to only take up one line, with ellipses or something, or we could set it so the minimum height is always two lines

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

I think that the table is a bit busy. I prefer it as an ordered list for the moment. However, I like the idea of adding headings like "In the last hour", "In the last day", and so on, whenever the user has selected to sort by date. Would you please try that?

Please keep the implementation separate from any sort order selector (#1829 (review)) because we only support sorting by date right now, so there is no need to couple these two PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants