Skip to content

feat: add indicator of disabled check#1290

Merged
VikaCep merged 3 commits into
mainfrom
feat/show-disabled-checks-hint
Oct 23, 2025
Merged

feat: add indicator of disabled check#1290
VikaCep merged 3 commits into
mainfrom
feat/show-disabled-checks-hint

Conversation

@VikaCep

@VikaCep VikaCep commented Oct 20, 2025

Copy link
Copy Markdown
Contributor

Add UI hint for checks disabled due to free tier overage

Closes issue #1261

Summary

Implements a visual indicator for checks that have been disabled due to free tier overage. When the disableReason field is populated on a check object, users will now see a clear orange "Disabled" badge with a tooltip explaining the reason.

image

@VikaCep VikaCep self-assigned this Oct 20, 2025
@VikaCep VikaCep requested a review from a team as a code owner October 20, 2025 18:37
@VikaCep VikaCep requested a review from w1kman October 20, 2025 18:37
@github-actions

github-actions Bot commented Oct 20, 2025

Copy link
Copy Markdown

Script size changes

Name +/- Main This PR Outcome
[module.js] +0.03% 2,417.74 kB 2,418.42 kB
[datasource/module.js] = 24.34 kB 24.34 kB

Totals

Name +/- Main This PR Outcome
[Scripts] +0.03% 2,442.08 kB 2,442.76 kB
[Non-script Assets] = 2,589.66 kB 2,589.66 kB
[All] +0.01% 5,031.74 kB 5,032.42 kB

Generated by 🚫 dangerJS against e109021

@ckbedwell ckbedwell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@The-9880 -- I'm cross-referencing the OAS Check schema (also 👏) and I noticed disableReason is always present in the schema which then showed me that reasonless checks have an empty string. My preference would be null, would that be a big deal to change?

What are the current strings we have that this field can be?

I'd also like for us to agree that the reason field is some sort of enum we can map to. Reasons for this are:

  1. i18n. The less English language strings we have coming from the API the easier it is for us to fully translate the app when we get round to it (which will be on the roadmap in FY27)
  2. Depending on the reason we can change the UI and potential messaging. In particular if we wanted to try and upsell if someone's checks were disabled because they hit the free tier limit.

@VikaCep -- works great. Maybe just add a simple test that if disableReason is present it gets rendered in the UI and if it is an empty string it doesn't?

@The-9880

Copy link
Copy Markdown

@The-9880 -- I'm cross-referencing the OAS Check schema (also 👏) and I noticed disableReason is always present in the schema which then showed me that reasonless checks have an empty string. My preference would be null, would that be a big deal to change?

What are the current strings we have that this field can be?

I'd also like for us to agree that the reason field is some sort of enum we can map to. Reasons for this are:

  1. i18n. The less English language strings we have coming from the API the easier it is for us to fully translate the app when we get round to it (which will be on the roadmap in FY27)
  2. Depending on the reason we can change the UI and potential messaging. In particular if we wanted to try and upsell if someone's checks were disabled because they hit the free tier limit.

@VikaCep -- works great. Maybe just add a simple test that if disableReason is present it gets rendered in the UI and if it is an empty string it doesn't?

Hey! I can absolutely change this to be null-able - i.e. the field is not even returned if it is empty.

The current only value we are using for this is FREE_LIMIT_EXCEEDED, which is a string but formatted like an enum identifier. Does this work for the app?

@ckbedwell

Copy link
Copy Markdown
Contributor

The current only value we are using for this is FREE_LIMIT_EXCEEDED, which is a string but formatted like an enum identifier. Does this work for the app?

Beautiful. Exactly what we want ❤️

@VikaCep can you make the changes so that we show a user-friendly message using something like a key/value map?

Comment thread src/page/CheckList/components/DisableReasonHint.test.tsx
@VikaCep

VikaCep commented Oct 22, 2025

Copy link
Copy Markdown
Contributor Author

@VikaCep can you make the changes so that we show a user-friendly message using something like a key/value map?

For sure! Just pushed some updates :)

@VikaCep VikaCep requested a review from ckbedwell October 22, 2025 15:00

@ckbedwell ckbedwell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@VikaCep VikaCep merged commit 1dc7f9c into main Oct 23, 2025
23 checks passed
@VikaCep VikaCep deleted the feat/show-disabled-checks-hint branch October 23, 2025 14:47
@sm-release-app sm-release-app Bot mentioned this pull request Oct 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants