Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ui): Enhance resource creation experience when limits are reached #19103

Merged
merged 7 commits into from
Jul 30, 2020

Conversation

mavarius
Copy link
Contributor

@mavarius mavarius commented Jul 27, 2020

Change rate-limited create asset experience.

OLD EXPERIENCE

Create asset button disabled when asset limit is reached for Dashboards, Tasks, & Buckets
image

Trying to create a Check fails with an error after going though the process of configuring it
image

Trying to create a Rule fails silently with only a console error
image

NEW EXPERIENCE

Create asset button is always enabled
image

When user clicks to create asset they are presented with an upgrade overlay
image

@mavarius mavarius changed the title feat(ui): create resource buttons WIP feat(ui): Enhance resource creation experience when limits are reached WIP Jul 29, 2020
@mavarius mavarius force-pushed the feat/create-resource-buttons branch from 95c948e to 6059f93 Compare July 29, 2020 18:08
@mavarius mavarius changed the title feat(ui): Enhance resource creation experience when limits are reached WIP feat(ui): Enhance resource creation experience when limits are reached Jul 29, 2020
@mavarius mavarius requested review from ebb-tide and a team July 29, 2020 18:08
@asalem1
Copy link
Contributor

asalem1 commented Jul 29, 2020

Just wanna say how great an enhancement this is

export const extractRulesMax = (limits: LimitsState): number => {
return get(limits, 'rules.maxAllowed') || Infinity
}
export const extractBlockedRules = (limits: LimitsState): string[] => {
return get(limits, 'rules.blocked') || []
}

export const extractEndpointsLimits = (limits: LimitsState): LimitStatus => {
return get(limits, 'endpoints.limitStatus')
Copy link
Contributor

Choose a reason for hiding this comment

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

should these return some defaults?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so. The defaults are set to LimitStatus.OK so there's always a value when these gets are called.

@@ -13,6 +13,7 @@ export type OverlayID =
| 'telegraf-output'
| 'switch-organizations'
| 'create-bucket'
| 'asset-limit'

export interface OverlayParams {
[key: string]: string
Copy link
Contributor

Choose a reason for hiding this comment

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

I really don't like that these params are actually required for the overlay to load properly, but they are not properly typed. I know you didn't write this, and I don't think you need to fix this in this PR, but it makes me worry :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should open another tech debt issue for this.

Copy link
Contributor

@ebb-tide ebb-tide left a comment

Choose a reason for hiding this comment

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

Thanks- I just had a few comments here and there but all are in the realm of style suggestions!

@mavarius mavarius force-pushed the feat/create-resource-buttons branch from 6059f93 to d715fff Compare July 30, 2020 15:54
@mavarius mavarius merged commit bc4c19d into master Jul 30, 2020
@jacobmarble jacobmarble deleted the feat/create-resource-buttons branch January 2, 2024 22:45
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