-
Notifications
You must be signed in to change notification settings - Fork 357
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
upcoming: [DI-20348] - Changes for Adding CloudPulseCustomSelect component and Integration #10807
upcoming: [DI-20348] - Changes for Adding CloudPulseCustomSelect component and Integration #10807
Conversation
…and its Integration
(CloudPulseSelectTypes.static === type && | ||
(!options || options.length === 0)) || (CloudPulseSelectTypes.dynamic === type && !apiV4QueryKey) | ||
) { | ||
staticErrorText = 'Pass pass either options or API query key'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a typo in this string?
* @param defaultSelectionProps - The props needed for getting the default selections | ||
* @returns | ||
*/ | ||
export const getDefaultSelectionsFromPreferencesAndPublishSelectionChanges = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we shorten the name of this function? Use the JSDoc comment instead to describe what it does.
* @param selectionChangeProps - The props needed for selecting the new filter and updating the global preferences | ||
*/ | ||
|
||
export const callSelectionChangeAndUpdateGlobalFilters = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here about shortening this function name.
@hkhalil-akamai , addressed the comments, please check if it is good now |
Coverage Report: ✅ |
@dwiley-akamai this is ready for review 👍 |
packages/manager/.changeset/pr-10807-upcoming-features-1724226251911.md
Outdated
Show resolved
Hide resolved
service_type: 'linode' | ||
}), | ||
dashboardFactory.build({ | ||
label: 'Dbaas Dashboard', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be cased as "DBaaS"?
packages/manager/src/features/CloudPulse/shared/CloudPulseCustomSelectUtils.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/shared/CloudPulseCustomSelectUtils.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/shared/CloudPulseCustomSelectUtils.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Utils/FilterBuilder.ts
Outdated
Show resolved
Hide resolved
packages/manager/src/features/CloudPulse/Utils/FilterBuilder.ts
Outdated
Show resolved
Hide resolved
@dwiley-akamai / @hkhalil-akamai - Addressed those comments including eslint issues as well, please check if it is good now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing feedback & apologies for the delay in approving.
Thank you for approval @hkhalil-akamai. @jaalah-akamai / @dwiley-akamai can we get second approval and merge if everything is fine |
@jaalah-akamai , we have enough approvals, please check if it is good for merging |
Description 📝
Changes for new component CloudPulseCustomSelect that helps to display various filters in the dashboard view and integration of custom select component with metrics API call.
Changes 🔄
List any change relevant to the reviewer.
Target release date 🗓️
9/3/24
Preview 📷
Include a screenshot or screen recording of the change
💡 Use
<video src="" />
tag when including recordings in table.How to test 🧪
Note- You might encounter flickering issues, that is caused due to preferences, that will be solved in the upcoming PR
As an Author I have considered 🤔
Check all that apply