Skip to content

Commit

Permalink
Inline create button with the title
Browse files Browse the repository at this point in the history
  • Loading branch information
bipuladh committed Apr 21, 2020
1 parent b0eea54 commit 870526e
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 33 deletions.
11 changes: 8 additions & 3 deletions frontend/__tests__/components/factory/list-page.spec.tsx
Expand Up @@ -39,8 +39,8 @@ describe(FireMan_.displayName, () => {
wrapper = shallow(<FireMan_.WrappedComponent resources={resources} />);
});

it('renders `PageHeading` if given `title`', () => {
expect(wrapper.find(PageHeading).exists()).toBe(false);
it('renders `title` if given `title`', () => {
expect(wrapper.find(PageHeading).props().title).toBe(undefined);

const title = 'My pods';
wrapper.setProps({ title });
Expand All @@ -53,7 +53,12 @@ describe(FireMan_.displayName, () => {

const createProps = { foo: 'bar' };
const button = wrapper
.setProps({ canCreate: true, createProps, createButtonText: 'Create Me!' })
.setProps({
canCreate: true,
createProps,
createButtonText: 'Create Me!',
title: 'Nights Watch',
})
.find('#yaml-create');

expect(
Expand Down
9 changes: 9 additions & 0 deletions frontend/public/components/_nav-title.scss
Expand Up @@ -24,6 +24,15 @@
padding-left: $grid-gutter-width;
padding-right: $grid-gutter-width;
}
&--row {
@media (max-width: $screen-xs-max) {
margin-bottom: 30px;
}
@media (min-width: $screen-sm) {
display: flex;
flex-direction: row;
}
}
&--detail {
border-bottom: 1px solid $color-grey-background-border;
flex-shrink: 0; // do not allow to shrink
Expand Down
51 changes: 25 additions & 26 deletions frontend/public/components/factory/list-page.jsx
Expand Up @@ -270,32 +270,31 @@ export const FireMan_ = connect(null, { filterList })(

return (
<>
{title && <PageHeading title={title} badge={badge} />}

This comment has been minimized.

Copy link
@rhamilto

rhamilto Apr 28, 2020

Member

Removing the title prop check caused https://bugzilla.redhat.com/show_bug.cgi?id=1828564

This comment has been minimized.

Copy link
@bipuladh

bipuladh Apr 28, 2020

Author Contributor

Are you sure?

This comment has been minimized.

Copy link
@rhamilto

rhamilto Apr 28, 2020

Member

Updated my original comment with the correct Bugzilla link. Sorry for the mistake. Too many tabs!

{/* Show help text above the filter bar if there's a create button. */}
{helpText && createLink && (
<p className="co-m-pane__help-text co-help-text">{helpText}</p>
)}
<div
className={classNames(
{ 'co-m-pane__filter-bar': createLink || helpText },
{
'co-m-pane__filter-bar--with-help-text': helpText && !createLink,
},
)}
>
{helpText && !createLink && (
<div className="co-m-pane__filter-bar-group co-m-pane__filter-bar-group--help-text">
{helpText}
</div>
)}
{createLink && <div className="co-m-pane__filter-bar-group">{createLink}</div>}
{!title && badge && (
<div className="co-m-pane__filter-bar-group co-m-pane__filter-bar-group--badge">
{badge}
</div>
)}
</div>
<div className="co-m-pane__body">
<PageHeading title={title} badge={badge} className="co-m-nav-title--row">
<div
className={classNames(
{ 'co-m-pane__filter-bar--inline': (createLink || helpText) && title },

This comment has been minimized.

Copy link
@rhamilto

rhamilto Apr 28, 2020

Member

filter-bar is now a misnomer as no filters ever appear in this <div>. Further, there are conditions when this results in an empty <div>. I think this is the result of changes to the filter bar in conjunction with your changes here. We probably need to revisit.

This comment has been minimized.

Copy link
@bipuladh

bipuladh Apr 28, 2020

Author Contributor

Yes this is causing the spacing issue in Installed Operators page. I will raise a PR ASAP.

{ 'co-m-pane__filter-bar--no-title': !title },
{ 'co-m-pane__filter-bar': !createLink || helpText },
{
'co-m-pane__filter-bar--with-help-text': helpText && !createLink,
},
)}
>
{createLink && (
<div className="co-m-pane__filter-bar-group co-m-pane__createLink">
{createLink}
</div>
)}
{!title && badge && (
<div className="co-m-pane__filter-bar-group co-m-pane__filter-bar-group--badge">
{badge}
</div>
)}
</div>
</PageHeading>
{helpText && <p className="co-m-pane__help-text co-help-text">{helpText}</p>}
<div className="co-m-pane__body co-m-pane__body--no-top-margin">
{inject(this.props.children, {
resources,
expand: this.state.expand,
Expand Down
9 changes: 7 additions & 2 deletions frontend/public/components/utils/headings.tsx
Expand Up @@ -87,6 +87,7 @@ export const PageHeading = connectToModel((props: PageHeadingProps) => {
badge,
getResourceStatus = (resource: K8sResourceKind): string =>
_.get(resource, ['status', 'phase'], null),
className,
} = props;
const extraResources = _.reduce(
props.resourceKeys,
Expand All @@ -109,6 +110,7 @@ export const PageHeading = connectToModel((props: PageHeadingProps) => {
{ 'co-m-nav-title--detail': detail },
{ 'co-m-nav-title--logo': props.icon },
{ 'co-m-nav-title--breadcrumbs': breadcrumbsFor && !_.isEmpty(data) },
className,
)}
style={style}
>
Expand All @@ -117,7 +119,9 @@ export const PageHeading = connectToModel((props: PageHeadingProps) => {
<SplitItem isFilled>
<BreadCrumbs breadcrumbs={breadcrumbsFor(data)} />
</SplitItem>
{badge && <SplitItem>{badge}</SplitItem>}
{badge && (
<SplitItem>{<span className="co-m-pane__heading-badge">{badge}</span>}</SplitItem>
)}
</Split>
)}
{showHeading && (
Expand All @@ -141,7 +145,7 @@ export const PageHeading = connectToModel((props: PageHeadingProps) => {
)}
</div>
)}
{!breadcrumbsFor && badge}
{!breadcrumbsFor && badge && <span className="co-m-pane__heading-badge">{badge}</span>}
{showActions && (
<div className="co-actions" data-test-id="details-actions">
{hasButtonActions && (
Expand Down Expand Up @@ -264,6 +268,7 @@ export type PageHeadingProps = {
badge?: React.ReactNode;
icon?: React.ComponentType<{ obj?: K8sResourceKind }>;
getResourceStatus?: (resource: K8sResourceKind) => string;
className?: string;
};

export type ResourceOverviewHeadingProps = {
Expand Down
15 changes: 13 additions & 2 deletions frontend/public/style/_common.scss
Expand Up @@ -40,6 +40,12 @@ $co-external-link-padding-right: 15px;
margin-right: 10px;
min-width: 0; // enables truncation of selected item, if needed
}
&--inline {
margin-left: auto;
}
&--no-title {
margin-bottom: 30px;
}
}

.co-m-pane__filter-bar,
Expand Down Expand Up @@ -105,6 +111,11 @@ $co-external-link-padding-right: 15px;
}
}

.co-m-pane__heading-badge {
line-height: 0;
margin-left: var(--pf-global--spacer--sm);
}

.co-m-pane__heading-link {
font-size: $font-size-base;
}
Expand Down Expand Up @@ -374,9 +385,9 @@ $co-external-link-padding-right: 15px;

.help-block {
margin-bottom: 0;
}
}

// Note: include .co-select-to-copy to enable single click selection of full text
// Note: include .co-select-to-copy to enable single click selection of full text
.co-truncate {
@include co-truncate;
}
Expand Down

0 comments on commit 870526e

Please sign in to comment.