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

add InvalidObject #45

Merged
merged 27 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
698ae1f
add InvalidObject
InsaneZein Oct 3, 2023
7b66abc
Merge branch 'main' into add-invalidobject
InsaneZein Oct 3, 2023
a9068b8
remove enzyme and sass; update styling class names; add isbeta prop
InsaneZein Oct 5, 2023
b1561d4
Merge branch 'add-invalidobject' of github.com:InsaneZein/react-compo…
InsaneZein Oct 5, 2023
e8f9cf9
remove enzyme dependencies
InsaneZein Oct 5, 2023
51e9b36
change test from js to tsx
InsaneZein Oct 5, 2023
45827e8
update snap
InsaneZein Oct 5, 2023
c6278ef
Update packages/module/src/InvalidObject/InvalidObject.tsx
InsaneZein Oct 5, 2023
c313408
Update packages/module/patternfly-docs/content/extensions/component-g…
InsaneZein Oct 5, 2023
d62af41
Update packages/module/patternfly-docs/content/extensions/component-g…
InsaneZein Oct 5, 2023
be4b7a9
Update packages/module/patternfly-docs/content/extensions/component-g…
InsaneZein Oct 9, 2023
819ff02
reimplement using EmptyState; move icon to folder
InsaneZein Oct 18, 2023
95849e2
Merge branch 'add-invalidobject' of github.com:InsaneZein/react-compo…
InsaneZein Oct 18, 2023
f6a1ce9
Merge branch 'main' into add-invalidobject
InsaneZein Oct 18, 2023
1d44d8a
lint
InsaneZein Oct 18, 2023
297ad24
Merge branch 'add-invalidobject' of github.com:InsaneZein/react-compo…
InsaneZein Oct 18, 2023
3d64665
fix merge issues
InsaneZein Oct 18, 2023
26adcbf
update snapshot
InsaneZein Oct 18, 2023
310b9fd
add notfoundicon index
InsaneZein Oct 19, 2023
df39ff2
Update packages/module/patternfly-docs/content/extensions/component-g…
InsaneZein Oct 19, 2023
63ebd31
Update packages/module/src/InvalidObject/index.ts
InsaneZein Oct 19, 2023
5d53c04
Update packages/module/patternfly-docs/content/extensions/component-g…
InsaneZein Oct 19, 2023
b4dcfc5
Update packages/module/patternfly-docs/content/extensions/component-g…
InsaneZein Oct 19, 2023
0bcc92f
remove unncessary styling
InsaneZein Oct 19, 2023
265f1a7
remove return because of lint
InsaneZein Oct 19, 2023
42d1d36
update snapshot
InsaneZein Oct 19, 2023
aabc7ab
Update packages/module/src/InvalidObject/InvalidObject.tsx
fhlavac Oct 19, 2023
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
689 changes: 569 additions & 120 deletions package-lock.json

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
# Sidenav top-level section
# should be the same for all markdown files
section: extensions
subsection: Component groups
# Sidenav secondary level section
# should be the same for all markdown files
id: Invalid object
# Tab (react | react-demos | html | html-demos | design-guidelines | accessibility)
source: react
# If you use typescript, the name of the interface to display props for
# These are found through the sourceProps function provided in patternfly-docs.source.js
propComponents: ['InvalidObject']
---

import InvalidObject from '@patternfly/react-component-groups/dist/dynamic/InvalidObject';


InsaneZein marked this conversation as resolved.
Show resolved Hide resolved
The **invalid object** component can be used to display an error message and "return to homepage" button when an error occurs.



InsaneZein marked this conversation as resolved.
Show resolved Hide resolved
## Component usage

### Example

InsaneZein marked this conversation as resolved.
Show resolved Hide resolved
```js file="./InvalidObjectExample.tsx"

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import InvalidObject from '@patternfly/react-component-groups/dist/dynamic/InvalidObject';
import React from 'react';

const InvalidObjectExample: React.FunctionComponent = () => (
<>
<InvalidObject />
</>
);
InsaneZein marked this conversation as resolved.
Show resolved Hide resolved

export default InvalidObjectExample;
9 changes: 9 additions & 0 deletions packages/module/src/InvalidObject/InvalidObject.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import InvalidObject from './InvalidObject';
import React from 'react';
import { render } from '@testing-library/react';

describe('InvalidObject component', () => {
test('should render', () => {
expect(render(<InvalidObject />)).toMatchSnapshot();
});
});
47 changes: 47 additions & 0 deletions packages/module/src/InvalidObject/InvalidObject.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { Button, Title } from '@patternfly/react-core';

import Icon404 from './icon-404';
import React from 'react';
import classNames from 'classnames';
Copy link
Collaborator

Choose a reason for hiding this comment

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

please, let's use clsx with a slightly smaller bundle size instead of classnames, current main already has that change - it's just renaming the function.

import { createUseStyles } from 'react-jss'

const useStyles = createUseStyles({
"invalidObject": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"invalidObject": {
invalidObject: {

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think the styling is needed with the EmptyState, it works even without it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I got rid of it. It does work without it.

display: "flex",
justifyContent: "center",
flexDirection: "column",
alignItems: "center",
textAlign: "center",
h1: { marginBottom: "30px" },
svg: { marginBottom: "30px" },
Copy link
Collaborator

@fhlavac fhlavac Oct 10, 2023

Choose a reason for hiding this comment

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

@InsaneZein the margins do not seem to work in the examples. Anyway, we could reimplement the component using PF EmptyState, what do you think? NotAuthorized component, already present in our repo is implemented in a similar way

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I can do that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@fhlavac I've reimplemented it to use EmptyState

button: { fontSize: "20px" }
},
"invalidObjectSorryText": { maxWidth: "70%" },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"invalidObjectSorryText": { maxWidth: "70%" },
invalidObjectSorryText: { maxWidth: "70%" },

})

export interface InvalidObjectProps {
/** Indicates if an additional "/beta" string should be added to when navigating back to the home screen. */
isBeta?: boolean
Copy link
Collaborator

Choose a reason for hiding this comment

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

@edonehoo This is the only prop that we have for this component.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still think rather than the isBeta prop we should allow customizing the props it affects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

so the whole href should be a prop?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@InsaneZein I would vote for being consistent with the NotAuthorized component

<Button variant="primary" component="a" href={toLandingPageUrl}>

}


const InvalidObject: React.FunctionComponent<InvalidObjectProps> = ({ isBeta }) => {
const classes = useStyles();
const invalidObjectClasses = classNames("pf-v5-l-page__main-section", "pf-v5-c-page__main-section", classes.invalidObject)
return (
<section className={invalidObjectClasses}>
<Title headingLevel="h1" size="3xl">
We lost that page
</Title>
<Icon404 />
<Title headingLevel="h1" size="xl" className={classes.invalidObjectSorryText}>
Let&apos;s find you a new one. Try a new search or return home.
</Title>
<Button variant="link" component="a" href={`${window.location.origin}${isBeta ? '/beta' : ''}`}>
Return to homepage
</Button>
</section>
);
};

export default InvalidObject;
Loading
Loading