added a subheading and copy button to the pagewithtabs component #1450
Conversation
ARADDCC002
commented
Aug 5, 2024
ARADDCC002
commented
| @@ -56,9 +56,11 @@ export default function DataCard() { | |||
| <PageWithTabs | |||
| showCopyButton | |||
| title={dataCard.name} | |||
| subheading={`ID: ${dataCard.id}`} | |||
There was a problem hiding this comment.
Do we wan't the same with Access Requests and Schemas?
There was a problem hiding this comment.
They don't use this component in the same way - the Schemas page for example just uses the static title "Schemas", the reason we need this change for entries is because they have dynamic names and IDs
| sourceModelId = '', | ||
| }: { | ||
| title: string | ||
| subheading?: string |
There was a problem hiding this comment.
I think this as an optional type looks like a typo?
There was a problem hiding this comment.
Not a typo - we don't always need a subheading eg the schemas page.
| tabs: PageTab[] | ||
| actionButtonTitle?: string | ||
| displayActionButton?: boolean | ||
| actionButtonOnClick?: () => void | ||
| requiredUrlParams?: ParsedUrlQuery | ||
| showCopyButton?: boolean | ||
| textToCopy?: string | ||
| titleToCopy?: string | ||
| subheadingToCopy?: string |
There was a problem hiding this comment.
Is it necessary to have extra ...ToCopy props since they're likely just repeats of existing props? I realise it's done this way already with the title?
There was a problem hiding this comment.
This is something I thought about, and you are right when it comes to the title for models and data cards, but it's slightly different for the subheading. In my current implementation, the subheading is "ID: my-model-123", but I don't want to copy to clipboard the bit that says "ID:". Equally, we don't want to hard code "ID:" into the PageWithTabs component, because subheadings might not always be IDs, they can be any string value.
frontend/src/common/PageWithTabs.tsx
Outdated
| <CopyToClipboardButton | ||
| textToCopy={subheadingToCopy ? subheadingToCopy : subheading} | ||
| notificationText='Copied to clipboard' | ||
| ariaLabel='copy to clipboard' | ||
| /> |
There was a problem hiding this comment.
Does this component need to be conditional on showCopyButton, like with the title page?
There was a problem hiding this comment.
Yes it does, good spot!
