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

Morecast: Add validation feedback for empty but required fields #3936

Closed
wants to merge 10 commits into from

Conversation

conbrad
Copy link
Collaborator

@conbrad conbrad commented Sep 16, 2024

Renders red border on cells when a value is required but is empty. For these cells, only when a user hovers them will it provide the prompt, "Enter a value" in a tooltip, so as not to overwhelm the datagrid.

Test Links:

Landing Page
MoreCast
Percentile Calculator
C-Haines
FireBat
FireBat bookmark
Auto Spatial Advisory (ASA)
HFI Calculator

@conbrad conbrad requested review from dgboss and brettedw September 16, 2024 20:58
@brettedw
Copy link
Collaborator

Not sure what is causing this, but sometimes locally I don't get a popup and the cursor flickers between pointer/text cursor, along with the mouse over row flickering as well. It seems to resolve when I move my cursor to the centre of the text field?

@conbrad
Copy link
Collaborator Author

conbrad commented Sep 16, 2024

Not sure what is causing this, but sometimes locally I don't get a popup and the cursor flickers between pointer/text cursor, along with the mouse over row flickering as well. It seems to resolve when I move my cursor to the centre of the text field?

Aah it's because forecast cell is rendered as part of a grid, I'm going to have to move the Tooltip to wrap the grid for ForecastCell

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.38%. Comparing base (6d12f7b) to head (c7eed2c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3936      +/-   ##
==========================================
+ Coverage   81.35%   81.38%   +0.02%     
==========================================
  Files         298      298              
  Lines       11421    11436      +15     
  Branches      546      552       +6     
==========================================
+ Hits         9292     9307      +15     
  Misses       2094     2094              
  Partials       35       35              

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

@conbrad conbrad requested a review from dgboss September 16, 2024 22:44
@dgboss
Copy link
Collaborator

dgboss commented Sep 16, 2024

I'm a little torn about always showing the red borders on cells that need data entry. A more typical pattern would be to validate once the submit/save button gets pressed and then display the border around cells that need it. It might be worth checking in with RP on this.

I'd also suggest updating the text in the warning toast message to say something about required fields being empty or something to that effect.

@conbrad
Copy link
Collaborator Author

conbrad commented Sep 16, 2024

I'm a little torn about always showing the red borders on cells that need data entry. A more typical pattern would be to validate once the submit/save button gets pressed and then display the border around cells that need it. It might be worth checking in with RP on this.

I'd also suggest updating the text in the warning toast message to say something about required fields being empty or something to that effect.

Perhaps the red border is a little aggressive for drawing attention to blank cells but I'd argue some sort of feedback before having the user attempt to submit is better than only seeing errors once you attempt to submit, to get the feedback as early as possible.

@rajpersram thoughts?

Copy link

@brettedw
Copy link
Collaborator

I'm a little torn about always showing the red borders on cells that need data entry. A more typical pattern would be to validate once the submit/save button gets pressed and then display the border around cells that need it. It might be worth checking in with RP on this.
I'd also suggest updating the text in the warning toast message to say something about required fields being empty or something to that effect.

Perhaps the red border is a little aggressive for drawing attention to blank cells but I'd argue some sort of feedback before having the user attempt to submit is better than only seeing errors once you attempt to submit, to get the feedback as early as possible.

@rajpersram thoughts?

Curious to hear @rajpersram thoughts. I'd say it's definitely an improvement over what we currently have but I also see Darren's point about a more typical pattern. I wonder if users might find the tooltips on every required input cell a little much, but maybe not because it sounds like they all just tab across inputs. Code looks good to me, curious what Raj has to say about UX

@rajpersram
Copy link
Member

I agree with only showing the red outline and note after an action (of submitting) has occured, as opposed to always showing it as red until a value is entered.

@conbrad
Copy link
Collaborator Author

conbrad commented Sep 17, 2024

Closing to save dev resources while we hackathon

@conbrad conbrad closed this Sep 17, 2024
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.

4 participants