Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Improve name of orderNotes flag for checkout-state provider. #2982

Closed
nerrad opened this issue Aug 7, 2020 · 2 comments
Closed

Improve name of orderNotes flag for checkout-state provider. #2982

nerrad opened this issue Aug 7, 2020 · 2 comments
Labels
block: checkout Issues related to the checkout block. type: enhancement The issue is a request for an enhancement.

Comments

@nerrad
Copy link
Contributor

nerrad commented Aug 7, 2020

While reviewing #2851, I left some feedback on choice of variable names for the account creation flag and noticed there's a similar problem with the naming around the orderNotes flag (added in #2877).

This issue is to address the orderNotes flag naming. It should better reflect that the property is a boolean. Maybe something like canLeaveOrderNotes. I realize it's more verbose, but it's immediately clear when parsing code it's purpose as opposed to orderNotes which on first glance I would assume is a list/collection of orderNotes.

@nerrad nerrad added type: enhancement The issue is a request for an enhancement. block: checkout Issues related to the checkout block. labels Aug 7, 2020
@haszari
Copy link
Member

haszari commented Aug 10, 2020

This issue is to address the orderNotes flag naming. It should better reflect that the property is a boolean. Maybe something like canLeaveOrderNotes. I realize it's more verbose, but it's immediately clear when parsing code it's purpose as opposed to orderNotes which on first glance I would assume is a list/collection of orderNotes.

FYI @nerrad - I think this orderNotes state is text; either empty (no order note) or containing user-supplied note text. In the API response it's serialised as customer_note. I agree though it maybe could be clearer and more consistent throughout.

Screen Shot 2020-08-10 at 5 24 37 PM

There is a boolean showOrderNotes block attribute, controlling whether the note UI is available to shopper. This might be clearer as allowOrderNotes.

@nerrad
Copy link
Contributor Author

nerrad commented Aug 10, 2020

OHHH, I totally misunderstood then, I should have clued in from the type on the default that it was not a boolean flag 🤦 . Some good suggestions in your comment about renaming but it's not as critical as what I originally thought with flags. So I'll close the issue.

@nerrad nerrad closed this as completed Aug 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: checkout Issues related to the checkout block. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants