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(v6): Update to PatternFly V6 Versions #54

Merged
merged 19 commits into from
May 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class MockFilterSidePanelExample extends React.Component {
const maxShowCount = 5;
const leeway = 2;
return (
<div style={{ width: '205px', border: '1px solid grey', paddingTop: '20px' }}>
<div style={{ width: '205px' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Removed the border and example padding so it's clear what's coming from <FilterSidePanel> and what's part of the example. Left the width because there is no width on <FilterSidePanel> and I'm assuming this panel goes into some parent/container with a constrained width, so this will mimic closer how the component will display. Ideally I'd probably drop this width, too, so you're just seeing the extension styles.

Just dropping a note here but this and the properties side panel seem to share some styling - they're narrow, probably have padding around them. And <FilterSidePanel> has padding and no width, and <PropertiesSidePanel> has a width and no padding. I'm guessing that's fine, but wondering if there could be some consistency there.

<FilterSidePanel id="filter-panel">
<FilterSidePanelCategory key="cat1">
<TextInput type="text" id="filter-text-input" placeholder="Filter by name" aria-label="filter text input" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,44 +26,42 @@ import OkIcon from '@patternfly/react-icons/dist/esm/icons/ok-icon';
import ExternalLinkAltIcon from '@patternfly/react-icons/dist/esm/icons/external-link-alt-icon';
import GlobeIcon from '@patternfly/react-icons/dist/esm/icons/globe-icon';

<div style={{ display: 'inline-block', padding: '15px', border: '1px solid grey' }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Took this wrapper off just so it's clear what style is coming from <PropertiesSidePanel> and what's just in the example code.

<PropertiesSidePanel>
<PropertyItem label="Operator Version" value="0.9.8 (latest)" />
<PropertyItem
label="Certified Level"
value={
<span>
<OkIcon style={{color: '--pf-t--global--color--status--success--default'}} /> Certified
</span>
}
/>
<PropertyItem label="Provider" value="Red Hat, Inc" />
<PropertyItem label="Health Index" value="A" />
<PropertyItem
label="Repository"
value={
<a href="https://quay.io/repository/redhat/prometheus-operator">
https://quay.io/repository/redhat/prometheus-operator
</a>
}
/>
<PropertyItem
label="Container Image"
value={
<a href="#">
0.22.2 <ExternalLinkAltIcon />
</a>
}
/>
<PropertyItem
label="Created At"
value={
<span>
<GlobeIcon /> Aug 23, 1:58pm
</span>
}
/>
<PropertyItem label="Support" value={<a href="#">Red Hat</a>} />
</PropertiesSidePanel>
</div>
<PropertiesSidePanel>
<PropertyItem label="Operator Version" value="0.9.8 (latest)" />
<PropertyItem
Comment on lines +30 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this should just be a <DescriptionList>. That's how it's called out in the figma design but I wasn't sure if that would work for the extension, depending on how it's used. If you used <DescriptionList> there would be no custom styles here with maaayyyybe the exception of this width

Everything else comes for free 💸

label="Certified Level"
value={
<span>
<OkIcon style={{color: '--pf-t--global--icon--color--status--success--default'}} /> Certified
Copy link
Contributor

Choose a reason for hiding this comment

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

updated this color token since the previous was just a generic success color and we have an icon specific success color.

</span>
}
/>
<PropertyItem label="Provider" value="Red Hat, Inc" />
<PropertyItem label="Health Index" value="A" />
<PropertyItem
label="Repository"
value={
<a href="https://quay.io/repository/redhat/prometheus-operator">
https://quay.io/repository/redhat/prometheus-operator
</a>
}
/>
<PropertyItem
label="Container Image"
value={
<a href="#">
0.22.2 <ExternalLinkAltIcon />
</a>
}
/>
<PropertyItem
label="Created At"
value={
<span>
<GlobeIcon /> Aug 23, 1:58pm
</span>
}
/>
<PropertyItem label="Support" value={<a href="#">Red Hat</a>} />
</PropertiesSidePanel>
```
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,20 @@
align-items: center;
}
.ws-react-e-catalog-item-header .catalog-item-header-pf-icon {
font-size: 60px;
max-height: 60px;
width: 60px;
font-size: var(--pf-t--global--icon--size--2xl);
max-height: var(--pf-t--global--icon--size--2xl);
width: var(--pf-t--global--icon--size--2xl);
}
.ws-react-e-catalog-item-header .catalog-item-header-pf-text {
margin-left: 20px;
margin-inline-start: var(--pf-t--global--spacer--md);
}
.ws-react-e-catalog-item-header .catalog-item-header-pf-title {
font-weight: 400;
margin-bottom: 0;
margin-top: 0;
font-weight: var(--pf-t--global--font--weight--body);
margin-block-end: 0;
margin-block-start: 0;
}
.ws-react-e-catalog-item-header .catalog-item-header-pf-subtitle {
color: #8b8d8f;
font-size: small;
margin-bottom: 0;
color: var(--pf-t--global--text--color--subtle);
font-size: var(--pf-t--global--font--size--body--sm);
margin-block-end: 0;
}
33 changes: 13 additions & 20 deletions packages/module/patternfly-docs/content/examples/catalogTile.css
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
.ws-react-e-catalog-tile .catalog-tile-pf:active, .ws-react-e-catalog-tile .catalog-tile-pf:hover, .ws-react-e-catalog-tile .catalog-tile-pf:focus, .ws-react-e-catalog-tile .catalog-tile-pf:visited {
color: inherit;
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems a little weird. The link tile has blue text/stuff in it by default (since it's a link), but this style is making all of that black on :active, :hover, :focus, :visited. So I just matched what I'm seeing in the browser for the current extension and updated the black color to use our default font color.

Here's the current extension - you'll likely need to update the href on it to some non-visited link to see the default state (I updated to href="#")

Screenshot 2024-05-02 at 5 45 33 PM

and on hover

Screenshot 2024-05-02 at 5 45 45 PM

color: var(--pf-t--global--text--color--regular);
text-decoration: none;
}
.ws-react-e-catalog-tile .catalog-tile-pf-header {
font-size: 16px;
font-weight: 400;
}
.ws-react-e-catalog-tile .catalog-tile-pf-header .catalog-tile-pf-title {
font-weight: 400;
}
.ws-react-e-catalog-tile .catalog-tile-pf-header .catalog-tile-pf-subtitle {
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this is a fair enhancement request for PF - a card subtitle. Then we could get rid of all of this, too.

color: #4D5258;
font-size: small;
color: var(--pf-t--global--text--color--subtle);
font-size: var(--pf-t--global--font--size--body--sm);
font-family: var(--pf-t--global--font--family--body);
font-weight: var(--pf-t--global--font--weight--body);
padding-block-start: var(--pf-t--global--spacer--xs);
}
.ws-react-e-catalog-tile .catalog-tile-pf-header .catalog-tile-pf-subtitle a {
color: var(--pf-t--global--text--color--link--default);
Expand All @@ -21,24 +17,21 @@
color: var(--pf-t--global--text--color--link--hover);
text-decoration: underline;
}
.ws-react-e-catalog-tile .pf-v6-c-card__actions {
padding-left: 5px;
}
.ws-react-e-catalog-tile .catalog-tile-pf-icon {
font-size: 40px;
height: 40px;
font-size: 37px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Updated to 37px per figma, but I don't know that it really matters if that needs to change to something else?

height: 37px;
max-width: 60px;
min-width: 40px;
min-width: 37px;
}
.ws-react-e-catalog-tile .catalog-tile-pf-badge-container {
display: flex;
flex: 1;
justify-content: flex-end;
margin-left: 10px;
margin-inline-start: var(--pf-t--global--spacer--sm);
}
.ws-react-e-catalog-tile .catalog-tile-pf-badge-container .catalog-tile-pf-badge {
font-size: 16px;
margin-left: 5px;
font-size: var(--pf-t--global--font--size--body--default);
margin-inline-start: var(--pf-t--global--spacer--xs);
}
Comment on lines 26 to 35
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what all goes in this element but depending on what all it supports, this may be able to be improved.

.ws-react-e-catalog-tile .catalog-tile-pf-body .catalog-tile-pf-description span {
display: -webkit-box;
Expand All @@ -50,5 +43,5 @@
-webkit-line-clamp: 1;
}
.ws-react-e-catalog-tile .example-ok-icon {
color: #4CB140;
color: var(--pf-t--global--icon--color--status--success--default);
Comment on lines -56 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see this in use, so maybe it can be removed?

}
Original file line number Diff line number Diff line change
@@ -1,50 +1,46 @@
.filter-panel-pf {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally this whole thing would be a <Form>. I think the markup would be different since currently each block of checkboxes is in its own <form> HTML element but it should give exactly the styling needed. Here's a rough codesandbox https://codesandbox.io/p/sandbox/gallant-bouman-wcjx32?file=%2Findex.tsx%3A31%2C15. The margins and everything in the v6 <Form> matched the figma design for this panel as far as I could tell.

margin: 0 0 30px;
padding: 0 15px;
}

.filter-panel-pf-category {
margin-top: 20px;
}

.filter-panel-pf-category:first-of-type {
margin-top: 0;
margin-block-start: 0;
margin-block-end: var(--pf-t--global--spacer--xl);
margin-inline: 0;
padding-inline: var(--pf-t--global--spacer--md);
display: flex;
flex-direction: column;
gap: var(--pf-t--global--spacer--lg);
}

.filter-panel-pf-category-title {
border: 0;
font-size: inherit;
margin: 0 0 8px 0;
font-weight: 700;
font-size: var(--pf-t--global--font--size--body--sm);
margin-block-start: 0;
margin-block-end: var(--pf-t--global--spacer--sm);
margin-inline: 0;
font-weight: var(--pf-t--global--font--weight--body--bold);
}

.filter-panel-pf-category-items {
margin-top: 0;
margin-bottom: 0;
margin-block: 0;
}

.filter-panel-pf-category-items .pf-v6-c-button.pf-m-link {
padding: 0;
}

.filter-panel-pf-category-item {
font-weight: 400;
margin-bottom: 0;
margin-top: 5px;
.filter-panel-pf-category-items > * + * {
margin-block-start: var(--pf-t--global--spacer--sm);
}

.filter-panel-pf-category-item:first-of-type {
margin-top: 0;
.filter-panel-pf-category-item {
font-weight: var(--pf-t--global--font--weight--body);
}

.filter-panel-pf-category-item .pf-v6-c-check__input {
margin-left: 0px;
.filter-panel-pf-category-item:first-of-type {
margin-block-start: 0;
}

.filter-panel-pf-category-item .item-icon {
padding-right: 3px;
padding-inline-end: var(--pf-t--global--spacer--xs);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this spacer/padding and the one on .item-count below could just come from regular space characters in the markup. The figma designs just use a space between the text and count.

}

.filter-panel-pf-category-item .item-count {
padding-left: 3px;
padding-inline-start: var(--pf-t--global--spacer--xs);
}
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
.properties-side-panel-pf {
Copy link
Contributor

Choose a reason for hiding this comment

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

mentioned this above but all of the styles in this stylesheet come with the <DescriptionList> component

width: 165px;
}

.properties-side-panel-pf-property {
margin-top: 24px; }
.properties-side-panel-pf-property:first-of-type {
margin-top: 0;
display: flex;
flex-direction: column;
gap: var(--pf-t--global--spacer--lg);
}

.properties-side-panel-pf-property-label {
font-weight: 700 !important;
font-size: 14px !important;
font-weight: var(--pf-t--global--font--weight--body--bold) !important;
font-size: var(--pf-t--global--font--size--body--sm) !important;
margin: 0 !important;
}

.properties-side-panel-pf-property-value {
font-size: 14px !important;
margin-top: 8px;
font-size: var(--pf-t--global--font--size--body--regular) !important;
margin-top: var(--pf-t--global--spacer--sm);
word-break: break-word;
}
42 changes: 27 additions & 15 deletions packages/module/patternfly-docs/content/examples/verticalTab.css
Original file line number Diff line number Diff line change
@@ -1,26 +1,37 @@
.ws-react-e-vertical-tabs .vertical-tabs-pf {
list-style: none;
margin: 0 0 30px;
margin-block-start: 0;
margin-block-end: 30px;
margin-inline-start: 0;
margin-inline-end: 0;
padding: 0;
display: flex;
flex-direction: column;
gap: var(--pf-t--global--spacer--xs);
}
.ws-react-e-vertical-tabs .vertical-tabs-pf .vertical-tabs-pf {
margin-bottom: 0;
margin-block-end: 0;
padding-block-start: var(--pf-t--global--spacer--xs);
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab {
margin-top: 4px;
position: relative;
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab > a {
color: initial;
color: var(--pf-t--global--text--color--regular);
display: inline-block;
font-size: 13px;
padding: 3px 6px 3px 15px;
font-size: var(--pf-t--global--font--size--body--default);
padding-block: var(--pf-t--global--spacer--xs);
padding-inline: var(--pf-t--global--spacer--sm);
vertical-align: top;
width: 100%;
word-break: break-word;
background-color: var(--pf-t--global--background--color--action--plain--default);
border-radius: var(--pf-t--global--border--radius--small);
margin-inline-start: var(--pf-t--global--spacer--md);
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab > a:hover, .ws-react-e-vertical-tabs .vertical-tabs-pf-tab > a:focus {
color: var(--pf-t--global--text--color--brand--hover);
color: var(--pf-t--global--text--color--regular);
background-color: var(--pf-t--global--background--color--action--plain--hover);
text-decoration: none;
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab > a.no-wrap {
Expand All @@ -32,22 +43,23 @@
text-overflow: ellipsis;
white-space: nowrap;
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab.active > a {
color: var(--pf-t--global--text--color--brand--clicked);
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab > a.disabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see any examples of a disabled link in this extension, but it was in figma, so added support for <a class="disabled">

color: var(--pf-t--global--text--color--on-disabled);
background: var(--pf-t--global--background--color--disabled--default);
pointer-events: none;
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab.active > a::before {
background: var(--pf-t--global--border--color--brand--clicked);
background: var(--pf-t--global--border--color--clicked);
content: " ";
left: 0;
inset-inline-start: 0;
position: absolute;
width: 3px;
width: var(--pf-t--global--border--width--extra-strong);
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab:first-of-type {
margin-top: 0;
margin-block-start: 0;
}
.ws-react-e-vertical-tabs .vertical-tabs-pf-tab > .vertical-tabs-pf > .vertical-tabs-pf-tab {
position: initial;
padding-left: 15px;
margin-inline-start: var(--pf-t--global--spacer--md);
}
.ws-react-e-vertical-tabs .vertical-tabs-pf.restrict-tabs .vertical-tabs-pf-tab {
display: none;
Expand Down
Loading
Loading