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

Crash details: Ability to swap addresses #1653

Open
wants to merge 6 commits into
base: nextjs
Choose a base branch
from

Conversation

roseeichelmann
Copy link
Contributor

@roseeichelmann roseeichelmann commented Jan 7, 2025

Associated issues

Closes cityofaustin/atd-data-tech#19961

Testing

URL to test:
Local or netlify

Steps to test:

  1. Go to a crash details page, scroll down to the primary address data card and see the new swap address button.
  2. Test clicking it, see the window that pops up, confirm that the address fields swap correctly on confirm.
  3. Is it weird that you can swap while editing a field?
  4. Scroll down and see your changes in the record history

Ship list

  • Check migrations for any conflicts with latest migrations in main branch
  • Confirm Hasura role permissions for necessary access
  • Code reviewed
  • Product manager approved

@roseeichelmann roseeichelmann added the WIP Work in progress label Jan 7, 2025
@roseeichelmann roseeichelmann changed the base branch from main to nextjs January 7, 2025 22:50
@@ -132,7 +132,7 @@ export default function UserDetails({
onClick={() => {
if (
window.confirm(
"Are you user you want to delete this user?"
"Are you sure you want to delete this user?"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wittle typo

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for vision-zero-nextjs ready!

Name Link
🔨 Latest commit 49a7017
🔍 Latest deploy log https://app.netlify.com/sites/vision-zero-nextjs/deploys/677f06ad197bf000086b11fa
😎 Deploy Preview https://deploy-preview-1653--vision-zero-nextjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

disabled={isMutating}
onClick={async () => {
if (
window.confirm(
Copy link
Contributor Author

@roseeichelmann roseeichelmann Jan 8, 2025

Choose a reason for hiding this comment

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

john and i talked about this and in the future we may create a reuseable confirm/cancel modal to use across the app but for now we are going with window.confirm

@roseeichelmann
Copy link
Contributor Author

should we do more auto uppercase on user input here/other places in the app?
image

Copy link
Member

@chiaberry chiaberry left a comment

Choose a reason for hiding this comment

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

It does feel weird that if you are editing and then click the swap addresses button, the input stays open with the previous value. i was trying to come up with a scenario where someone would accidentally duplicate the primary address instead of swapping, but that feels far fetched and the values are in the change log, so nothing would really be lost.

Visually i want the lines to line up better, but its like this in prod now anyway, so mvp mvp 🚢
Screenshot 2025-01-09 at 4 50 54 PM

@chiaberry
Copy link
Member

I just saw the comment about auto all caps -- and if its thats how it coming from CRIS, then I think so yes, we should make it match the rest of the data. But I can be convinced otherwise!

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.

Crash details: Ability to swap addresses
2 participants