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

fix(CatalogTile): Edit target selector #3291

Merged
merged 1 commit into from Nov 11, 2019

Conversation

@rebeccaalpert
Copy link
Member

rebeccaalpert commented Nov 8, 2019

Adjusted selector for gradient effect.

@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 8, 2019

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

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 8, 2019

Codecov Report

Merging #3291 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3291   +/-   ##
=======================================
  Coverage   67.44%   67.44%           
=======================================
  Files         892      892           
  Lines       24867    24867           
  Branches     2141     2141           
=======================================
  Hits        16772    16772           
  Misses       7091     7091           
  Partials     1004     1004
Flag Coverage Δ
#misc 95.45% <ø> (ø) ⬆️
#patternfly3 69.3% <ø> (ø) ⬆️
#patternfly4 64.78% <ø> (ø) ⬆️

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 03d0285...7b6d67b. Read the comment docs.

Copy link
Contributor

jcaianirh left a comment

lgtm

position: absolute;
right: 0;
width: 50%;
text-align: right;

This comment has been minimized.

Copy link
@jeff-phillips-18

jeff-phillips-18 Nov 8, 2019

Member

Better to have:

.catalog-tile-pf-description {
 .truncated {
    overflow: hidden;
    position: relative;

    &::after {
      background: linear-gradient(to right, rgba(255, 255, 255, 0), rgba(255, 255, 255, 1) 75%);
      bottom: 0;
      color: transparent;
      content: ".";
      position: absolute;
      right: 0;
      width: 50%;
      text-align: right;
    }
  }
}

However, I don't believe we want this only applied when truncated is set. The idea is for this to auto truncate when necessary and the caller need not provide truncated text.

This comment has been minimized.

Copy link
@rebeccaalpert

rebeccaalpert Nov 8, 2019

Author Member

The class is applied whenever the truncation function is called. The user doesn't have to provide truncated text.

This comment has been minimized.

Copy link
@rebeccaalpert

rebeccaalpert Nov 8, 2019

Author Member

Updated the commit with the change.

This comment has been minimized.

Copy link
@jeff-phillips-18

jeff-phillips-18 Nov 9, 2019

Member

Why the check for isTruncated at all?

This comment has been minimized.

Copy link
@rebeccaalpert

rebeccaalpert Nov 11, 2019

Author Member

If all the descriptions have these styles, even really short descriptions that don't need truncation get faded out. This is a due to a difference between the old card and the new card. The old card had a fixed height applied based on the amount of space the text took up, so it wasn't an issue. I didn't think it was a great idea to use the fixed height thing with PF4 card styles.

This comment has been minimized.

Copy link
@jeff-phillips-18

jeff-phillips-18 Nov 11, 2019

Member

OK, this PR is fine by me. The height issue might be a problem if applications try to show tiles with differing header text lengths (causing wrapping). In PF3, that was taken into consideration and the height allowed for the descriptive text auto adjusted for it. Something to watch out for.

Adjusted selector for gradient effect.
@rebeccaalpert rebeccaalpert force-pushed the rebeccaalpert:catalogtileedits branch from 44ad032 to 7b6d67b Nov 8, 2019
@patternfly-build

This comment has been minimized.

Copy link
Contributor

patternfly-build commented Nov 8, 2019

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

Copy link
Contributor

jcaianirh left a comment

lgtm

@jeff-phillips-18 jeff-phillips-18 merged commit 16bf30c into patternfly:master Nov 11, 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.