feat: Add Serial number to Option in LookupField #695
Conversation
konradhopek-zen
force-pushed
the
khopek/PDSC-218
branch
from
ffcda2d to
d042280
Compare
konradhopek-zen
requested review from
kristkvch-zen and
zendesk-kamil-zachradnik
konradhopek-zen
force-pushed
the
khopek/PDSC-218
branch
from
d042280 to
4ce2678
Compare
| <> | ||
| {option.name} | ||
| <Option.Meta> | ||
| <Span hue="grey">SN: {option.serialNumber}</Span> |
There was a problem hiding this comment.
(nit) Just a quick question regarding this: should this SN string be translated? We will probably want this to appear differently depending on the language.
konradhopek-zen
force-pushed
the
khopek/PDSC-218
branch
from
4ce2678 to
68674c0
Compare
| export interface TicketFieldOptionObject { | ||
| name: string; | ||
| value: string; | ||
| serialNumber?: string; |
There was a problem hiding this comment.
Where is this property coming from?
It doesn't seem to be a property of ticket-field-option which is the target of this type. If this is only valid for Service Catalog and ITAM please use a union.
There was a problem hiding this comment.
yes it was from Service Catalog, did union.
There was a problem hiding this comment.
ITAMAssetOptionObject only makes sense within the service-catalog context. It does not make sense in the context of new-request-form which is the other consumer of this shared package.
We don't want to have application specific logic in the shared packages so we need to find a way to avoid having to do this. Ideally the union would be placed on the service-catalog side.
| {t( | ||
| "cph-theme-ticket-fields.lookup-field.serial-number-label", | ||
| "SN:" | ||
| )}{" "} |
There was a problem hiding this comment.
You can probably pass the number as a parameter for the translation string, which saves you the trouble of doing this. Check this example from guide-client
It will also require changes in the yml file, just like this
There was a problem hiding this comment.
ok, done. Thanks for this.
konradhopek-zen
force-pushed
the
khopek/PDSC-218
branch
from
68674c0 to
8e209ad
Compare
| export interface TicketFieldOptionObject { | ||
| name: string; | ||
| value: string; | ||
| serialNumber?: string; |
There was a problem hiding this comment.
ITAMAssetOptionObject only makes sense within the service-catalog context. It does not make sense in the context of new-request-form which is the other consumer of this shared package.
We don't want to have application specific logic in the shared packages so we need to find a way to avoid having to do this. Ideally the union would be placed on the service-catalog side.
konradhopek-zen
force-pushed
the
khopek/PDSC-218
branch
from
af8f509 to
85bd8d0
Compare
konradhopek-zen
requested review from
BrunoBFerreira,
luis-almeida and
zendesk-kamil-zachradnik
luis-almeida
left a comment
luis-almeida
left a comment
There was a problem hiding this comment.
Thanks for addressing the feedback! 👍🏼
|
🎉 This PR is included in version 4.16.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Description
Add a serial number to the dropdown in the end-user view to improve clarity and make it easier for users to distinguish between similar items:https://www.figma.com/design/2smlj3LkdOUZceDq4Q6shY/Service-catalog---after-GA?node-id=1009-37813&p=f&t=uqlgQpizgTmVMYmx-0
Jira
https://zendesk.atlassian.net/browse/PDSC-218
Screenshots
Checklist