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(react-catalog-view-extension): Added catalog view extension and three components #3145

Merged
merged 27 commits into from Oct 24, 2019

Conversation

@rebeccaalpert
Copy link
Member

rebeccaalpert commented Oct 15, 2019

Address: #3159

Heads up for @jcaianirh.

@rebeccaalpert rebeccaalpert added the PF4 label Oct 15, 2019
@rebeccaalpert

This comment has been minimized.

Copy link
Member Author

rebeccaalpert commented Oct 15, 2019

PF4 docs build locally; not sure why the test is failing.

Copy link
Contributor

jcaianirh left a comment

@rebeccaalpert Were we going to create a react-extensions structure that followed: /pf4/react-extenstions/react-catalog-view-extension/...

@rebeccaalpert

This comment has been minimized.

Copy link
Member Author

rebeccaalpert commented Oct 15, 2019

@jcaianirh - Yes, that's the current structure.

Copy link
Member

dlabrecq left a comment

Is there an issue for this? Perhaps you can attach a screen shot?

Curious what a "catalog" item header is, what is it meant to look like, variations, etc. What makes it a candidate for an extension?

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Oct 17, 2019

Codecov Report

Merging #3145 into master will increase coverage by 0.09%.
The diff coverage is 85.29%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3145      +/-   ##
==========================================
+ Coverage   69.04%   69.13%   +0.09%     
==========================================
  Files         859      868       +9     
  Lines       23643    23780     +137     
  Branches     1896     1935      +39     
==========================================
+ Hits        16324    16441     +117     
- Misses       6359     6360       +1     
- Partials      960      979      +19
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.3% <ø> (ø) ⬆️
#patternfly4 68.3% <85.29%> (+0.22%) ⬆️
Impacted Files Coverage Δ
...tternfly-4/react-core/src/components/Card/Card.tsx 92.85% <ø> (ø) ⬆️
...ension/src/components/PropertiesSidePanel/index.ts 100% <100%> (ø)
...ly-4/react-core/src/components/Tooltip/Tooltip.tsx 84.74% <100%> (+0.26%) ⬆️
...components/CatalogItemHeader/CatalogItemHeader.tsx 53.84% <53.84%> (ø)
...4/react-catalog-view-extension/src/helpers/util.ts 66.66% <66.66%> (ø)
...onents/PropertiesSidePanel/PropertiesSidePanel.tsx 75% <75%> (ø)
...on/src/components/CatalogTile/CatalogTileBadge.tsx 76.92% <76.92%> (ø)
...nsion/src/components/VerticalTabs/VerticalTabs.tsx 90% <90%> (ø)
...on/src/components/VerticalTabs/VerticalTabsTab.tsx 90% <90%> (ø)
...rc/components/PropertiesSidePanel/PropertyItem.tsx 90.9% <90.9%> (ø)
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e98180...89c6dba. Read the comment docs.

@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:catalogitemheader branch from d33a1cc to df1de26 Oct 17, 2019
@dlabrecq

This comment has been minimized.

Copy link
Member

dlabrecq commented Oct 17, 2019

Can you please attach an issue?

@jeff-phillips-18

This comment has been minimized.

Copy link
Member

jeff-phillips-18 commented Oct 17, 2019

Seem the PR title/description needs to be updated to include the rest of the catalog components.

Copy link
Member

dlabrecq left a comment

The PR only mentions including a catalog item header. However, I see that vertical tabs was just added (8 hrs ago), after the initial PR was created. Wondering if we should limit the scope of this PR to the original catalog item header, considering this is such a large change?

@@ -0,0 +1,95 @@
.vertical-tabs-pf {

This comment has been minimized.

Copy link
@dlabrecq

dlabrecq Oct 18, 2019

Member

Don't know what the policy is for extension packages, but we don't typically deliver CSS? There is some inline styles included with react-inline-edit-extension, but that is still referencing PatternFly selectors.

@rebeccaalpert rebeccaalpert changed the title feat(Catalog Item Header): Added catalog item header component in new extension feat(react-catalog-view-extension): Added catalog view extension and three components Oct 18, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Oct 18, 2019

PatternFly-React preview: https://patternfly-react-pr-3145.surge.sh

@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:catalogitemheader branch 5 times, most recently from dff04ac to dc5e41d Oct 18, 2019
@redallen

This comment has been minimized.

Copy link
Contributor

redallen commented Oct 22, 2019

@jeff-phillips-18

This comment has been minimized.

Copy link
Member

jeff-phillips-18 commented Oct 22, 2019

Will the tiles still support faded text truncation as in the PF3 extensions?

image

@jeff-phillips-18

This comment has been minimized.

Copy link
Member

jeff-phillips-18 commented Oct 22, 2019

The multiple icon badges seem too close together:
image

@rebeccaalpert

This comment has been minimized.

Copy link
Member Author

rebeccaalpert commented Oct 22, 2019

It's because the CSS was getting stripped out. I'm pushing a change that should fix it. I've been working with Zack on this.

@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:catalogitemheader branch from 81add97 to 3eaf7a7 Oct 24, 2019
Copy link
Contributor

jcaianirh left a comment

lgtm

@rebeccaalpert rebeccaalpert dismissed dlabrecq’s stale review Oct 24, 2019

Dana said this has a pass on outside reviewers since we're going to make iterative improvements in OpenShift as needed prior to release.

@jcaianirh jcaianirh merged commit 20a981b into patternfly:master Oct 24, 2019
8 checks passed
8 checks passed
ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: build_integration Your tests passed on CircleCI!
Details
ci/circleci: build_pf3_docs Your tests passed on CircleCI!
Details
ci/circleci: build_pf4_docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: test_jest_other Your tests passed on CircleCI!
Details
ci/circleci: test_jest_pf4 Your tests passed on CircleCI!
Details
ci/circleci: upload_docs Your tests passed on CircleCI!
Details
@rebeccaalpert rebeccaalpert deleted the rebeccaalpert:catalogitemheader branch Oct 24, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Oct 24, 2019

Your changes have been released in:

  • @patternfly/react-catalog-view-extension@1.1.0
  • @patternfly/react-core@3.120.0
  • @patternfly/react-docs@4.16.0
  • @patternfly/react-inline-edit-extension@2.12.9
  • demo-app-ts@3.9.1
  • @patternfly/react-table@2.24.9
  • @patternfly/react-topology@2.11.0
  • @patternfly/react-virtualized-extension@1.3.9

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.