-
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
1082: Prefill card form from query params #1089
1082: Prefill card form from query params #1089
Conversation
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.
Tested on chrome! Works fine to me :)
If i'm right there shouldn't be additional effort for the starting date (my pr) since this is an extension
cardBlueprint.setValue(header, value) | ||
}) | ||
setCardBlueprints([cardBlueprint]) | ||
setSearchParams() |
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.
Hm it might be better to clean the searchParams after creating barcodes?
On the other hand f.e. a logout doesn't trigger cleaning.
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 it makes sense to remove the searchParams imediately to not accidentally refill the form with values. So I'd keep it.
@@ -42,6 +44,24 @@ type CreateCardsFormProps = { | |||
|
|||
const CreateCardsForm = ({ region, cardBlueprints, setCardBlueprints }: CreateCardsFormProps) => { | |||
const projectConfig = useContext(ProjectConfigContext) | |||
// http://localhost:3000/cards/add?name=Thea+Test& |
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.
remove comment
@@ -69,6 +88,15 @@ export class CardBlueprint { | |||
return this.expirationDate !== null && this.expirationDate.isAfter(today) | |||
} | |||
|
|||
setExpirationDate = (value: string) => { | |||
if (value.length === 0) return null |
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.
if (value.length === 0) return null | |
if (value.length === 0) return |
i think this should be fine without null
A few examples:
I'd really like to add tests for this, but I think we need to migrate to jest for this.
Mind that you can now cheesily set the Adressvalues, when passing them as query params, but I don't think we need to prohibit that, as it (1) adds more complexity to this and (2) will be unknown to our users as long as we don't tell them :D