-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: Elevator Screens #529
Conversation
ef230bd
to
dd5effe
Compare
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.
One comment, looks good otherwise!
@@ -15,20 +15,22 @@ import ScreenDetail from "Components/ScreenDetail"; | |||
import { sortScreens } from "../../util"; | |||
import { useUpdateAnimation } from "Hooks/useUpdateAnimation"; | |||
import classNames from "classnames"; | |||
import _ from "lodash"; |
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 thought we were using lodash/fp
?
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.
Ahh yes thank you. (Although I dislike how bad it is at inferring types 😅)
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 agree this is a notable downside of FP and maybe something we should have considered (if we'd had a better sense for it at the time) in our conversations about which "utility library" to use. I tend to avoid Lodash entirely and use vanilla JS where feasible, since built-in classes and functions have generally excellent types (except for you, JSON.parse
, you're not invited to the party).
Now that we can make elevator screens, we need to make them work in Screenplay. This involves a few things:
stop_id
inBuilder
. Did this with a/facilities
lookupiframe
s display correctlyDeploying this to dev-green so it can be seen in a deployed environment.