-
Notifications
You must be signed in to change notification settings - Fork 3
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
adds substantial completion date tooltip #1454
base: 19257_show_substantial_completion_date_plus_advanced_search
Are you sure you want to change the base?
adds substantial completion date tooltip #1454
Conversation
@@ -315,6 +315,7 @@ export const useColumns = ({ hiddenColumns }) => { | |||
{ | |||
headerName: "Substantial completion date", | |||
field: "substantial_completion_date", | |||
description: "The earliest confirmed start date of a project phase that has a simple phase name of Complete (Complete or Post-Construction)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tillyw Could you update the description to say Post-construction
so it matches the capitalization in the data dictionary? I made a typo and then copy-pasta'd into every issue involving this date. I just fixed those in the issues. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ and I copied this text for the tooltip in my branch im working on
@@ -315,6 +315,7 @@ export const useColumns = ({ hiddenColumns }) => { | |||
{ | |||
headerName: "Substantial completion date", | |||
field: "substantial_completion_date", | |||
description: "The earliest confirmed start date of a project phase that has a simple phase name of Complete (Complete or Post-construction)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌 @chiaberry @tillyw Could y'all coordinate for someone to make this description into a variable to hold this string so we can reference it in this PR and in #1456? We may end up adding this in the Data Dictionary too so it could help us update the definition if we ever need to in the future.
I looked in the codebase, and maybe we could add a new file in src/constants
to hold tooltip text. What do y'all think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add this to my branch, trying to decide what to name the file, projects.js
sound okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it here: a60747a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, @chiaberry!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chiaberry sorry i merged your entire branch into this one when i should have just merged this one commit! i'll just make sure to merge this one last out of the substantial completion date PRs
Associated issues
fixes cityofaustin/atd-data-tech#19259
based off pr #1453
I believe the placement of this definition is centralized since we can import this helper object into the project view, but please let me know if there would be a better place to store it!
Testing
URL to test:
https://deploy-preview-1454--atd-moped-main.netlify.app/
Steps to test:
Ship list