add fix for hide category toggle#2662
Conversation
Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
Philip-Carneiro
force-pushed
the
fix-hide-category-toggle
branch
from
c7a817a to
d909932
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Signed-off-by: Philip Colares Carneiro <philip.colares@gmail.com>
| <Title headingLevel="h3" size="lg"> | ||
| {categoryTitle} | ||
| </Title> | ||
| {performanceViewEnabled && !hasMultipleSourceCategories(catalogSources) && ( |
There was a problem hiding this comment.
one quick question - why we need !hasMultipleSourceCategories(catalogSources) check here because the outer condition to render this whole div cover theisSingleCategory?
feels like redundant check, no?
| title="No performance data available in selected category" | ||
| title={ | ||
| isSingleCategory | ||
| ? 'No performance data available.' |
There was a problem hiding this comment.
I don't think we need a period after this line to be consistent with the other title
| ? 'No performance data available.' | |
| ? 'No performance data available' |
| return orderedLabels; | ||
| }; | ||
|
|
||
| export const hasMultipleSourceCategories = (catalogSources: CatalogSourceList | null): boolean => { |
There was a problem hiding this comment.
Isn't this util kind of similar to getActiveSourceLabels? Its just that return string[] rather than boolean. Can't we reuse that?
| initIntercepts({ | ||
| sources: [mockCatalogSource({ labels: ['Provider one'] })], | ||
| hasValidatedModels: false, | ||
| }); | ||
| setupFilteredModelsIntercept({ returnModelsForFilters: false }); | ||
| modelCatalog.visit(); | ||
|
|
||
| modelCatalog.togglePerformanceView(); |
There was a problem hiding this comment.
These initial setup was repeated for the these 3 tests so can we have it in beforeEach()?
2 participants
Description
In Model performance view when only a single category is available:
Before:
After:
How Has This Been Tested?
This was manually tested, also added tests in cypress
Merge criteria:
DCOcheck)ok-to-testhas been added to the PR.If you have UI changes