CONSOLE-3305: add DetailsPage and associated components #14

rhamilto · 2023-04-27T13:10:25Z

action menu working actions menu page header update tests updated index.ts

Option to have action menu icon after text

update

…ls-to-use-monorepo-common" This reverts commit a406b86, reversing changes made to 364a2d740a5daa1c5528af118e7fd3d4dff64ba7.

fhlavac

Looks good to me, thank you for the docs examples @rhamilto 🙂

rhamilto

A few comments to clarify significant changes.

packages/module/package.json

packages/module/patternfly-docs/content/extensions/extended-components/examples/DetailsPage.md

packages/module/src/DetailsPage/DetailsPage.tsx

packages/module/src/DetailsPageHeader/DetailsPageHeader.tsx

rhamilto · 2023-05-05T13:10:05Z

@fhlavac, thanks for the review!

@jhadvig, PTAL.

jhadvig

/lgtm

packages/module/patternfly-docs/content/extensions/extended-components/examples/DetailsPage.md

packages/module/src/DetailsPageHeader/DetailsPageHeader.tsx

Hyperkid123

I have a few comments.

packages/module/src/DetailsPageHeader/utils/Breadcrumbs.tsx

packages/module/src/DetailsPage/DetailsPage.tsx

packages/module/src/DetailsPageHeader/DetailsPageHeader.tsx

packages/module/src/DetailsPageHeader/utils/ActionButtons.tsx

packages/module/src/DetailsPageHeader/utils/ActionMenu.tsx

packages/module/src/HorizontalNav/HorizontalNav.tsx

rhamilto · 2023-05-11T12:58:44Z

@Hyperkid123, thanks for the review and comments! I believe all your comments relate to the code as it exists in https://github.com/openshift/dynamic-plugin-sdk (what I'm doing in this PR is migrating it over). I don't know the entire history since @vidyanambiar did the work, but it certainly can be reconsidered here. Just want to make sure everyone is on the same page and the history is considered. cc: @jhadvig, @florkbr

Hyperkid123 · 2023-05-15T07:20:41Z

@rhamilto Yup I am ware of its origin. We want to make sure that we do not transfer any product-specific code to this generic repository. Component API changes are OK and to be expected to some degree. Functionality should be preserved.

Hyperkid123 · 2023-05-15T07:36:21Z

packages/module/src/HorizontalNav/HorizontalNav.tsx

+        if (location?.pathname && navigate) {
+          const currentPathName = location.pathname;
+          if (params?.selectedTab) {
+            navigate(currentPathName.replace(params.selectedTab, eventKey as string), {


Hmm. The Tab component does support a href prop, but we can't customize the root component.

I think using the Link from the router package component is optimal but the Tab component support is currently not good enough. If we use it as a child node of the Tab title prop, we will require some extra styling. Can you experiment with it to see if we can use it? Let me know if its too much work for too little reward. We can skip it if it is.

For starters, I'm unclear how to deal with routing in general in the context of patternfly-docs. For example, how do I make use of react-router-dom hooks? Adding them to an example does not work. Perhaps it's not possible?

Yeah, that is fair. I see we have two options. We either endorses the react-router-dom library for routing and expect the consuming app to have the router context setup, or we extend the API to not just only use click events to handle navigation but also proved the option to configure root elements of components that support the usage of link tags.

The reason why it's not working in example docs is because the Router context was never set up.

@Hyperkid123, I defer to those of you with more experience on the subject. What do y'all think? @jhadvig, @florkbr, @fhlavac, @dlabaj

We do try to keep this repo as generic as possible. I would personally go with providing the API to modify the components rather than enforcing any libraries. After all, the long-term goal of this repo is to be product agnostic.

That said, there are other files that already use the router hooks etc.

Is it OK if we say, is OK for now, but we will make a breaking change in the near future that would change the component API to allow this generic approach?

I agree that the intent of this repo is for reusable UI components that extend PF in a meaningful way. The flows may be specific to Redhat.

@dlabaj can we reuse the PF beta mechanism for releasing these components? That way we can really think through the API changes going forward. I'd imagine this would be valuable to other components we contribute in the future.

Something like:

packages/module/src/DetailsPageHeader/DetailsPageHeader.tsx

jhadvig

Really nice work @rhamilto
Thanks @Hyperkid123 @dlabaj @florkbr for guidance here 👍

/lgtm

Hyperkid123 · 2023-06-05T09:23:25Z

@florkbr @dlabaj can we use the beta flag for modules in PF extensions?

florkbr · 2023-06-08T19:24:54Z

@Hyperkid123 @rhamilto I performed a quick test locally and it appears we can add the beta: true clause in DetailsPage.md metadata and it shows up in the docs UI:

I'm looking at the promotion of the truncate component in PF5 as an example: patternfly/patternfly-react@34c1688

Hyperkid123 · 2023-06-09T07:30:56Z

@florkbr @rhamilto cool. If we can add that we can merge the initial work. I'll think about how we could ensure the flexibility of the components.

rhamilto · 2023-06-09T12:13:16Z

@Hyperkid123 @rhamilto I performed a quick test locally and it appears we can add the beta: true clause in DetailsPage.md metadata and it shows up in the docs UI:

I'm looking at the promotion of the truncate component in PF5 as an example: patternfly/patternfly-react@34c1688

Done. Thanks, @florkbr.

github-actions · 2023-06-13T18:00:58Z

🎉 This PR is included in version 1.0.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

rhamilto · 2023-06-14T17:37:00Z

packages/module/src/HorizontalNav/HorizontalNav.tsx

+import { Tabs, Tab, TabProps as PfTabProps, TabTitleText } from '@patternfly/react-core';
+import React from 'react';
+import { useNavigate, useLocation } from 'react-router-dom';
+import '@patternfly/react-styles/css/utilities/Spacing/spacing.css';


D'oh! This should have been removed. cc: @florkbr, @jhadvig, @Hyperkid123, @dlabaj

I will open a PR to fix as this blocks me from making use of DetailsPage in https://github.com/openshift/crontab-plugin

PR to resolve: #22

rhamilto · 2023-06-15T15:30:39Z

packages/module/src/HorizontalNav/HorizontalNav.tsx

+          ouiaId={tab?.ouiaId}
+          key={tab.eventKey}
+        >
+          <div className="pf-u-m-md">{tab.children}</div>


<div className="pf-u-m-md"> is probably a mistake here as it is possible to not want padding on the tab children.

Opened a PR to address: #26

rhamilto force-pushed the CONSOLE-3305 branch 8 times, most recently from ba42874 to 9f476e0 Compare May 1, 2023 20:25

rhamilto changed the title ~~[WIP] CONSOLE-3305: add DetailsPage and associated components~~ CONSOLE-3305: add DetailsPage and associated components May 3, 2023

rhamilto changed the title ~~CONSOLE-3305: add DetailsPage and associated components~~ [WIP] CONSOLE-3305: add DetailsPage and associated components May 3, 2023

vidyanambiar and others added 15 commits May 4, 2023 13:15

horizontal tabs

114229d

Horizontal navigation with routing

7001104

Page header

f1876f9

action menu working actions menu page header update tests updated index.ts

Option to have action menu icon after text

a09d43a

Option to have action menu icon after text

Option to add icon before of after page heading title

cbd1a49

detailspage & storybook

31098b5

update

update stories based on changes to header component

71aa44b

Fix test performance

9339af9

Add Resource Summary

535f6e1

Add action buttons to list view

0c82ee9

Use API Extractor on distributable packages

0710c90

Pull AnyObject and EitherNotBoth helpers from monorepo common

cfb2f84

Revert "Merge pull request patternfly#196 from florkbr/update-lib-uti…

59fd2c1

…ls-to-use-monorepo-common" This reverts commit a406b86, reversing changes made to 364a2d740a5daa1c5528af118e7fd3d4dff64ba7.

Support CommonJS build for all dist packages

0b8b5f1

Reorganize files to match new repo

a4a701b

rhamilto force-pushed the CONSOLE-3305 branch from 5877767 to 1cee07b Compare May 4, 2023 17:16

Hyperkid123 requested a review from fhlavac May 5, 2023 07:48

fhlavac approved these changes May 5, 2023

View reviewed changes

rhamilto commented May 5, 2023

View reviewed changes

rhamilto changed the title ~~[WIP] CONSOLE-3305: add DetailsPage and associated components~~ CONSOLE-3305: add DetailsPage and associated components May 5, 2023

jhadvig approved these changes May 10, 2023

View reviewed changes

packages/module/patternfly-docs/content/extensions/extended-components/examples/DetailsPage.md Show resolved Hide resolved

packages/module/src/DetailsPageHeader/DetailsPageHeader.tsx Show resolved Hide resolved

Hyperkid123 requested changes May 11, 2023

View reviewed changes

rhamilto changed the title ~~CONSOLE-3305: add DetailsPage and associated components~~ [WIP] CONSOLE-3305: add DetailsPage and associated components May 11, 2023

Hyperkid123 reviewed May 15, 2023

View reviewed changes

rhamilto commented May 15, 2023

View reviewed changes

packages/module/src/DetailsPageHeader/DetailsPageHeader.tsx Outdated Show resolved Hide resolved

rhamilto force-pushed the CONSOLE-3305 branch from 349db35 to b03556a Compare May 16, 2023 21:03

Refactor for docs

2afcb15

rhamilto force-pushed the CONSOLE-3305 branch from 1d7f69e to 2afcb15 Compare May 31, 2023 14:24

rhamilto changed the title ~~[WIP] CONSOLE-3305: add DetailsPage and associated components~~ CONSOLE-3305: add DetailsPage and associated components May 31, 2023

jhadvig approved these changes Jun 5, 2023

View reviewed changes

Hyperkid123 self-requested a review June 9, 2023 07:29

Add beta to DetailsPage.md

8b53cf2

Hyperkid123 approved these changes Jun 9, 2023

View reviewed changes

Hyperkid123 merged commit 6d9512b into patternfly:main Jun 9, 2023

rhamilto deleted the CONSOLE-3305 branch June 9, 2023 12:43

github-actions bot added the released on @prerelease label Jun 13, 2023

rhamilto commented Jun 14, 2023

View reviewed changes

rhamilto commented Jun 15, 2023

View reviewed changes

github-actions bot mentioned this pull request Apr 21, 2025

The automated release is failing 🚨 #692

Closed

CONSOLE-3305: add DetailsPage and associated components #14

CONSOLE-3305: add DetailsPage and associated components #14

Uh oh!

Conversation

rhamilto commented Apr 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fhlavac left a comment

Choose a reason for hiding this comment

Uh oh!

rhamilto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhamilto commented May 5, 2023

Uh oh!

jhadvig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Hyperkid123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhamilto commented May 11, 2023

Uh oh!

Hyperkid123 commented May 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jhadvig left a comment

Choose a reason for hiding this comment

Uh oh!

Hyperkid123 commented Jun 5, 2023

Uh oh!

florkbr commented Jun 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hyperkid123 commented Jun 9, 2023

Uh oh!

rhamilto commented Jun 9, 2023

Uh oh!

github-actions bot commented Jun 13, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rhamilto Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

rhamilto commented Apr 27, 2023 •

edited

Loading

Hyperkid123 commented May 15, 2023 •

edited

Loading

florkbr commented Jun 8, 2023 •

edited

Loading

rhamilto Jun 15, 2023 •

edited

Loading