add RHEL, Ansible, and OpenShift to services dropdown #2855
Conversation
InsaneZein
requested review from
Hyperkid123,
ryelo,
karelhala and
florkbr
as code owners
| import { Divider } from '@patternfly/react-core/dist/esm/components/Divider'; | ||
| import { Text, TextVariants } from '@patternfly/react-core/dist/esm/components/Text'; |
There was a problem hiding this comment.
Do not use esm imports. Use /dynamic
| import { Divider } from '@patternfly/react-core/dist/esm/components/Divider'; | ||
| import { Text, TextVariants } from '@patternfly/react-core/dist/esm/components/Text'; | ||
| import ChromeLink from '../ChromeLink'; | ||
| import { Button } from '@patternfly/react-core'; |
There was a problem hiding this comment.
Use absolute import path for dependency sharing optimization purposes.
| </TabTitleText> | ||
| } | ||
| /> | ||
| <Text className="pf-v5-u-pl-lg pf-v5-u-pr-0 pf-v5-u-pt-0 pf-v5-u-pb-sm" component={TextVariants.p}> |
There was a problem hiding this comment.
Can you extract the new links into a separate component? The JSX is starting to be kind of long.
| </Button> | ||
| </ChromeLink> | ||
| <ChromeLink href="/insights/dashboard#SIDs=&tags="> | ||
| <Button variant="link" component="a" className="pf-v5-u-pl-lg"> |
There was a problem hiding this comment.
This will throw an invalid DOM nesting error. You can't have a tag within a tag.
Warning: validateDOMNesting(...): <a> cannot appear as a descendant of <a>
There was a problem hiding this comment.
i got rid of the that caused these errors
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2855 +/- ##
==========================================
+ Coverage 61.74% 61.75% +0.01%
==========================================
Files 204 205 +1
Lines 4540 4542 +2
Branches 856 856
==========================================
+ Hits 2803 2805 +2
Misses 1726 1726
Partials 11 11
|
|
/retest |
| @@ -23,7 +23,7 @@ describe('<AllServicesDropdown />', () => { | |||
| it('should close all services dropdown in link matches current pathname', () => { | |||
| function checkMenuClosed() { | |||
| cy.get('.pf-v5-c-menu-toggle__text').click(); | |||
| cy.contains('All services').should('exist'); | |||
There was a problem hiding this comment.
There is also a cy.contains('All services').should('not.exist'); bellow. This check has to be changed because now its always true. The test does not verify if the dropdown was closed or not.
| <ChromeLink href="/ansible/ansible-dashboard" className="pf-v5-u-pl-lg pf-v5-u-pb-sm"> | ||
| Red Hat Ansible Platform | ||
| </ChromeLink> | ||
| <ChromeLink href="/insights/dashboard#SIDs=&tags=" className="pf-v5-u-pl-lg pf-v5-u-pb-sm"> |
There was a problem hiding this comment.
I don't think we want to apply any filter, or even use the empty ones in the URL.
The URL should be /insights. Otherwise, it will do a full page refresh. The app is not registered for such a pathname.
| const PlatformServiceslinks = () => { | ||
| return ( | ||
| <> | ||
| <ChromeLink href="/ansible/ansible-dashboard" className="pf-v5-u-pl-lg pf-v5-u-pb-sm"> |
There was a problem hiding this comment.
The URL should be /ansible. Otherwise, it will do a full page refresh. The is not registered for such a pathname.
| cy.contains('Favorites').click(); | ||
| cy.contains('Test section').click(); | ||
| cy.contains('Test link').click(); | ||
| cy.contains('All services').should('not.exist'); |
There was a problem hiding this comment.
You want to verify the dropdown is closed. YOu are now looking for a different string.
cy.contains('View all').should('not.exist');
|
/retest |
for RHCLOUD-32660