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 Search Date Range Frontend #1265

Conversation

tylerbarna
Copy link
Member

Fresh PR, as the old PR (#1101) was from prior to some updates that made merging difficult. This includes all the frontend changes made in that PR, along with the associated existing issues, namely the anchoring of the details dropdown (see #1171 for fix-in-progress) as well as an issue where selecting a specific fuzzy date range doesn't properly update the url parameters - rather, the fuzzy date range lags by one (ie, if I select option A, the page will reload without applying option A. If I then proceed to select option B, option A will now be present in the url)

@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Attention: 54 lines in your changes are missing coverage. Please review.

Comparison is base (5becc82) 5.58% compared to head (8e83ce0) 5.52%.
Report is 220 commits behind head on main.

❗ Current head 8e83ce0 differs from pull request most recent head 077ead1. Consider uploading reports for the commit 077ead1 to get more accurate results

Additional details and impacted files
@@           Coverage Diff            @@
##            main   #1265      +/-   ##
========================================
- Coverage   5.58%   5.52%   -0.07%     
========================================
  Files         99     112      +13     
  Lines       2433    2663     +230     
  Branches     297     307      +10     
========================================
+ Hits         136     147      +11     
- Misses      2295    2514     +219     
  Partials       2       2              
Files Coverage Δ
app/routes/circulars._index.tsx 0.00% <0.00%> (ø)

... and 58 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tylerbarna
Copy link
Member Author

reposting the UI concept from the previous PR
image

@tylerbarna
Copy link
Member Author

This is also dependent on trussworks/react-uswds#2537

@tylerbarna tylerbarna changed the title Circulars Date Range Frontend Circulars Search Date Range Frontend Aug 21, 2023
had to use absolute positioning, still have issue with z-index of footer vs z-index of content
@lpsinger
Copy link
Member

@tylerbarna, would you please post a few screenshots?

@tylerbarna
Copy link
Member Author

tylerbarna commented Aug 22, 2023

@tylerbarna, would you please post a few screenshots?

sure:

image
image
image

I think the UI of the dropdown buttons could use some work, and I'd also prefer if the left side of the dropdown was aligned with the dropdown button, but that seems to potentially be a limitation of the buttonGroup; there's also the date format issue we've discussed before that still needs to be addressed in react-uswds

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.

  • Rely on passive HTML form data behavior rather than dynamically constructed form data whenever possible. Instead of actively, programmatically constructing the date range params for the form submission from React states, give the fuzzy date range inputs names and values that result in appropriate form data. They can be either radio buttons or submit buttons; in either case, you can have multiple buttons with the same name but different values, and the button that is clicked is the one whose value is present in the form data.
  • Use the default appearances of react-uswds components whenever possible. The USWDS components are intended to have typography and (generous) spacing that provides a consistent look and feel. Use CSS utility classes and custom styles sparingly. This will also fix the issue that the design looks quite cramped due to too little whitespace.
  • Display the current date range limits (if any are set) in the dropdown button in its collapsed form.

@lpsinger
Copy link
Member

lpsinger commented Sep 6, 2023

@tylerbarna, any updates on this?

@tylerbarna
Copy link
Member Author

@tylerbarna, any updates on this?

Hey Leo, my parents were visiting last week, so I'm getting caught up; will be working on addressing your comments today

@tylerbarna
Copy link
Member Author

May need to tweak the styling further, but I reverted the buttons to their standard styling
image
image

Also added a basic implementation of the dynamic button label, which definitely needs refinement.
image
image
image

@tylerbarna
Copy link
Member Author

tylerbarna commented Sep 6, 2023

  • Rely on passive HTML form data behavior rather than dynamically constructed form data whenever possible. Instead of actively, programmatically constructing the date range params for the form submission from React states, give the fuzzy date range inputs names and values that result in appropriate form data. They can be either radio buttons or submit buttons; in either case, you can have multiple buttons with the same name but different values, and the button that is clicked is the one whose value is present in the form data.

@lpsinger Could you expand on this? Would we no longer be utilizing a react state to store/read the start and end date parameters in the url? @dakota002 and I spent some time in a huddle earlier brainstorming how to potentially alter the repetitive onClick behaviour for the fuzzy date ranges, but we were encountering some trouble when trying to make alterations such that the following conditions were satisfied:

  1. The fuzzy times and user-defined dates use the same url parameters
  2. The fuzzy times were human readable in the url
  3. The fuzzy times were dynamic (ie if, in February, I copy the url of a search that specifies only results from the past month, navigating to that same url in September will return results from August, not January)

made mtd and ytd button labels reactive, also made it so there is not an undefined if only one date is defined
moved the submission of fuzzy dates out to a separate button, which fixes the issue of the lagging url parameter for whatever reason. will try radio button implementation.
the radio buttons are not within the button, but they seem to be more functional. the visual component needs to be refined, however.
@lpsinger
Copy link
Member

@tylerbarna, you raise some good points that I hadn't considered. I am thinking about this.

@tylerbarna
Copy link
Member Author

@lpsinger I am tinkering with using radio buttons; after spending some time working on it with Courey, we were able to address that parameter lagging by one submission issue by setting the parameter by selecting the item and then having a separate "submit" button, which is a design choice I would think compliments using radio buttons. I still need to see how the radio buttons work with the custom date ranges, however. Is this along the lines of what you were thinking for the radio buttons?

@lpsinger
Copy link
Member

Is this along the lines of what you were thinking for the radio buttons?

I thought that the radio buttons would have been a way to avoid needing a React state for the fuzzy date, so no, not really. Don't worry about that too much for now. I'll take a fresh look at this PR at my next opportunity.

However, from the screen shots I still see that the button content is extremely compressed and crowded. Please use the default spacing for button groups as much as possible. We aim to use default USWDS styling as much as possible so that whitespace, fonts, and layout is consistent across the entire UI.

@tylerbarna
Copy link
Member Author

However, from the screen shots I still see that the button content is extremely compressed and crowded. Please use the default spacing for button groups as much as possible. We aim to use default USWDS styling as much as possible so that whitespace, fonts, and layout is consistent across the entire UI.

Yeah, I've been trying to figure out how to have the width of the details dropdown button be dynamic to prevent it from feeling so crowded, but the default behaviour seems to be overflowing out of view rather than widening the button

following the lead of Courey, I moved the date range filter to a tab below the search bar so we have a more consistent design language when it comes to advanced searches
custom date range is now a radio button that can be used to expand the start and end date range picker. The radio buttons are no longer hidden when the custom range is expanded.
@lpsinger
Copy link
Member

However, from the screen shots I still see that the button content is extremely compressed and crowded. Please use the default spacing for button groups as much as possible. We aim to use default USWDS styling as much as possible so that whitespace, fonts, and layout is consistent across the entire UI.

Yeah, I've been trying to figure out how to have the width of the details dropdown button be dynamic to prevent it from feeling so crowded, but the default behaviour seems to be overflowing out of view rather than widening the button

@Courey, I think that @tylerbarna could use some CSS help here.

@tylerbarna
Copy link
Member Author

@lpsinger given the addition of other advanced search options, do you think it might be better to move the date range filter to also be in an advanced search filters tab?

image

image

image

upon clicking a different radio button, the custom date range div will be collapsed. The date range selector is still organized in the advanced search filters tab method, but can switch back to the dropdown with some tweaking.
updated dropdown button content to use the style developed for the advanced search filters (so they are now radio buttons and the custom date range div expands or collapses with the selection of the custom radio). However, the flex row and flex columns are no longer aligned correctly with this change, and the issue with the dropdown button label having an overflow persists.
@tylerbarna
Copy link
Member Author

@lpsinger added commits that revert to a dropdown menu but using the radio buttons; they still need to be realigned or have the dropdown menu width altered, and the issue with the button label overflowing persists. #1110 and #1366 are also still present

tylerbarna added a commit to tylerbarna/gcn.nasa.gov that referenced this pull request Oct 5, 2023
…. This will be a new PR due to previous PR being stale
@tylerbarna
Copy link
Member Author

Closed due to staleness, replaced by #1482

@tylerbarna tylerbarna closed this Oct 5, 2023
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.

2 participants