feat: updated status badge appearance and organisation #936
Conversation
| return ( | ||
| <Stack> | ||
| <Toggletip | ||
| content={ | ||
| <div> | ||
| {description} | ||
| {docsLink && ( | ||
| <> | ||
| {` `} | ||
| <TextLink href={docsLink} external variant="bodySmall"> | ||
| Read more | ||
| </TextLink> | ||
| </> | ||
| )} | ||
| </div> | ||
| } | ||
| > | ||
| <button className={styles.badgeLink} type="button"> | ||
| <Badge | ||
| text={ | ||
| <Stack gap={0.5} alignItems={`center`}> | ||
| <div>{text}</div> | ||
| <Icon name={`info-circle`} size="sm" /> | ||
| </Stack> | ||
| } | ||
| color={color} | ||
| /> | ||
| </button> | ||
| </Toggletip> | ||
| </Stack> | ||
| ); |
There was a problem hiding this comment.
This is so that regardless where the badge appears a user can check what it actually means for them. I've made it a toggletip because the content contains a link (if it were hover only it would be inaccessible to keyboard).
| @@ -73,16 +72,12 @@ export const ChooseCheckType = ({ checkType, checkTypeGroup, disabled }: ChooseC | |||
| {description} | |||
| </Text> | |||
| )} | |||
| {status ? <CheckStatusBadge status={status} /> : <BadgePlaceholder />} | |||
There was a problem hiding this comment.
This became redundant now I've added the status to the section title of the form.
| {checkType === CheckType.Browser && ( | ||
| <Alert severity="info" title=""> | ||
| <div> | ||
| Browser checks are in private preview. During the preview they are free to use: test executions will | ||
| not be billed.{' '} | ||
| <TextLink | ||
| href="https://grafana.com/docs/grafana-cloud/cost-management-and-billing/understand-your-invoice/synthetic-monitoring-invoice/" | ||
| external={true} | ||
| > | ||
| Read more | ||
| </TextLink> | ||
| </div> | ||
| </Alert> | ||
| )} |
There was a problem hiding this comment.
I didn't like that this became decoupled from the status when using useCheckTypeOptions. We had an incident a while back (which prompted me to create the aforementioned hook) where one part of the UI said the check was in one state and another part of the UI mentioned another state, so I've removed it from here and co-located this information with the check's status.
| <FormLayout.Section | ||
| label={checkTypeStep1Label[checkType]} | ||
| fields={[`job`, ...defineCheckFields]} | ||
| status={status} |
There was a problem hiding this comment.
I found the alert banner quite overwhelming and distracting from making a browser check (especially as there was no way to dismiss it and it was on every step of the form).
| @@ -31,7 +31,7 @@ const getStyles = (theme: GrafanaTheme2) => ({ | |||
| }), | |||
| }); | |||
|
|
|||
| const hideTelemetryForTypes = [CheckType.Scripted, CheckType.MULTI_HTTP]; | |||
| const hideTelemetryForTypes = [CheckType.Scripted, CheckType.MULTI_HTTP, CheckType.Browser]; | |||
There was a problem hiding this comment.
I thought I'd add this whilst I was there (I was considering adding the alert about them being free to this component but decided against it in the end).
| @@ -80,7 +79,6 @@ export const CHECK_TYPE_GROUP_OPTIONS: CheckTypeGroupOption[] = [ | |||
| description: `Monitor the availability and performance of a website using a real browser.`, | |||
| value: CheckTypeGroup.Browser, | |||
| icon: `globe`, | |||
| status: CheckStatus.PRIVATE_PREVIEW, | |||
There was a problem hiding this comment.
Similar problem to above where I wanted all of the associated information colocated so it is easier to reason with.
| @@ -40,6 +41,10 @@ export const ChooseCheckGroup = () => { | |||
| const CheckGroupCard = ({ group }: { group: CheckTypeGroupOption }) => { | |||
| const styles = useStyles2(getStyles); | |||
| const { isOverCheckLimit, isOverScriptedLimit } = useLimits(); | |||
| const checkOptions = useCheckTypeOptions().filter((option) => option.group === group.value); | |||
| const checksWithStatus = checkOptions.filter((option) => option.status); | |||
| const shouldShowStatus = checksWithStatus.length === checkOptions.length; | |||
There was a problem hiding this comment.
This actually resulted in the Public Preview badge appearing for Scripted checks which I thought was a good thing!
ckbedwell
merged commit 18527a7
into
browser-checks-public-preview-badge
* feat: add private preview badge to browser checks * feat: add banner on free usage for browser tests private preview * feat: updated status badge appearance and organisation (#936) --------- Co-authored-by: Chris Bedwell <christopher.bedwell@grafana.com>
Because we are on a tight schedule I thought I'd make some of the change suggestions I had for the original branch (if we didn't have a hard deadline I'd have been more polite 😅)
I'll add the explanations for the changes in a self-review of the PR.