Add Back Button to Final "Add Machine" Screen

[ryan-spencer1220]

Hey Ryan, thanks for the feedback on the issue! Totally understand your reasoning. Feel free to merge or close if you think this isn’t necessary, I just wanted to give it a shot. If you do decide to implement, et me know if you’d like any adjustments!

Description:
This PR implements the feature discussed in #594.

Changes made:

• Added a back button to the top-left of the final "Add Machine" modal screen.
• Uses FontAwesome6 with platform-specific icons (arrow-left for Android, chevron-left for iOS).
• Pressing the back button returns the user to the machine selection screen.
• Pressing the cancel button returns the user to the location details screen.

Rationale:
Provides a clear, consistent way for users to go back and select a different machine without implying that they are cancelling the entire process. This maintains the existing large Cancel button for its current flow, but repurposing it to send the user back to the location screen.

Linked Issue:
Fixes #594

[RyanTG]

Thanks! We'll review, but it might be a little while. Our current task is a pretty significant feature addition, so we're trying to focus on that for a bit. Once that work is merged we'll review this PR.

[RyanTG]

If you're interested, the only adjustment I'm seeing right now is to be more precise with the positioning of the back arrow (both vertically and horizontally) and the "add blah blah" vertical positioning. We have a few screens where we're basically faking the header, so you can use those as guidance. We've tried many solutions for this over the years, and right now the current solution seems good. The issue is that some phones have different top inset heights (due to notches and whatnot).

You can see the implementations here https://github.com/search?q=repo%3Apinballmap%2Fpbm-react%20useSafeAreaInsets&type=code

And basically, it's to use

const insets = useSafeAreaInsets();
const topMargin = insets.top;

And then that topMargin will be calculated more consistently per device. The machineDetails screen has a similar design as this modal, with text aligned next to arrow. And we had to do some bonkers PixelRatio calculations to account for users who use giant text sizes on their phones (which a lot of users do use, so it's good to always test for that).

Basically, the goal here would be to have the back arrow in the same spot on locationDetails, findMachine, and the confirm modal.

If you don't have capacity to do it, that's ok!

[RyanTG]

Hi @ryan-spencer1220 I’m assuming you don’t have capacity, so I will take it up.

[ryan-spencer1220]

Hey @RyanTG,

Sorry for the slow reply! I just wrapped up finals week for my CS program at Oregon State, so things have been a little busy. I was actually reviewing your comment this morning. If you haven’t already made this adjustment, I’m happy to jump in and handle it.

I did have one question: I noticed that FindMachine is one of a few class components in the app. Is there a specific reason for that, or are they just left over from when components were being converted from classes to functions?

The reason I ask is that useSafeAreaInsets() needs to be called inside a functional component. It looks like withSafeAreaInsets() works fine as an alternative for classes, but I was wondering if it might make more sense to convert FindMachine to a functional component instead. I’m happy to go either way if you have a preference.

[RyanTG]

Ah, I forgot it was a class component. That is just a relic of old code. We have been migrating to functional components on an as-needed basis. So yeah, we would want to migrate this one, if you want to do it.

[ryan-spencer1220]

Sure! I should have some time later today to work on this.

[ryan-spencer1220]

Hey @RyanTG,

I converted that class component to a functional component and implemented the useSafeAreaInsets() hook you mentioned. Let me know if you’d recommend any other changes. Thanks!

[bpoore]

Hi @ryan-spencer1220 and @RyanTG, apologies I initially missed this PR. Appreciate the work you are putting into this!

This screen in particular is our most complicated screens. It is used in two different places, adding a machine to a location as well as adding machines when submitting a new location. On this branch, you will see the experience to add machines when looking to submit a location isn't working. In addition to not working, the MultiSelectRow component is being re-rendered for every machine when pressing on a machine. In order to optimize for performance, you can see we were extending from the base PureComponent class (which allows only the pressed machine row to be re-rendered). To port over that same optimization here, we will need to be reaching for memoization. Depending on how y'all want to proceed, we could back out the conversion to function components currently or we will need to work to get this working properly for the "suggest location" context.

Thank you for your work here, I really appreciate it!

[ryan-spencer1220]

Hi @bpoore, thanks for taking a look at this!

That makes sense, I can look into implementing memoization. It should be doable to recreate the same functionality in a functional component.

Quick question: is there a preferred way or environment to test the “add machine to location” and “suggest location” flows? I’ve just been using my personal account in the simulator, which works fine for UI tweaks, but probably isn’t ideal for validating functional updates.

[bpoore]

Hi @ryan-spencer1220, thank you for asking! We do have a staging server that can be fired up. To interact with that rather than the prod API, in your .env update EXPO_PUBLIC_API_URL to https://pbmstaging.com/api/v1. Be sure to restart your local server if you have one running locally when you make this change. You can coordinate with @RyanTG as he will be the one to turn it on / off.

[RyanTG]

Yep, just let me know! We keep it off most of the time. But all actual map edits during testing should be done on it.

[ryan-spencer1220]

Thanks @bpoore!

@RyanTG I have some time this weekend to work on this if you're able to switch the staging server on.

[RyanTG]

Ok, it's on! npx expo start --clear to make sure it's getting the latest .env

When you're done, or taking a long break, please let me know and I'll turn it off. It's not the best setup right now because it shares resources with the production site. But no pressure and we can leave it up for a while.

You can also hop into our Discord if you want to chat about things. Link is on the website home page, or the about page of the app.