Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Bootstrap Treeview V2#437

Merged
jeff-phillips-18 merged 1 commit intopatternfly:masterfrom
priley86:treeview-testing
Aug 30, 2016
Merged

Bootstrap Treeview V2#437
jeff-phillips-18 merged 1 commit intopatternfly:masterfrom
priley86:treeview-testing

Conversation

@priley86
Copy link
Copy Markdown
Member

@priley86 priley86 commented Aug 25, 2016

Description

Upgrade Patternfly to Bootstrap Treeview V2

  • V2 changes of patternfly-bootstrap-treeview are here
  • New tests added can be reviewed here

Changes:

  • adds checkboxes and hierarchal checks to test page, and updates check icons per MGMTUX-403
  • adds highlight select test, and updates highlight styles per PTNFLY-1732
  • adds load-load, prevent unselect, and disabled nodes tests after feedback from @skateman

Comment thread bower.json Outdated
"eonasdan-bootstrap-datetimepicker": "~4.17.37",
"moment": "~2.14.1",
"patternfly-bootstrap-treeview": "~1.0.0"
"patternfly-bootstrap-treeview": "patternfly"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will update this PR with 2.0.0 after it is published and #1 is merged.

padding: 0 10px;
text-overflow: ellipsis;
white-space: nowrap;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

added per @skateman suggestion

Comment thread less/bootstrap-treeview.less Outdated
margin-right: 5px;
}
.node-disabled {
color:silver;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should use a color from the patternfly palette.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jeff-phillips-18 thanks - I will update it shortly.

@priley86
Copy link
Copy Markdown
Member Author

@jeff-phillips-18 I've updated the PR to reflect these changes. Thanks for the review!

I will update this PR once more after #1 is merged/published.

Comment thread less/color-variables.less Outdated
@color-pf-red: @color-pf-red-100;
@color-pf-white: #fff; No newline at end of file
@color-pf-white: #fff;
@color-pf-silver: #c0c0c0; No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there an existing color we could use? Adding a color to the color palette means updating a bunch of other places. @kybaker do you have any suggestions?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Silver sounds fantastic, but it'd be more accurate if we use one of our -blacks.

If no black match the color we are looking for then create a new black-xxx. looping @kybaker

@kybaker
Copy link
Copy Markdown
Contributor

kybaker commented Aug 29, 2016

@priley86 The color you specified works. We need to make a note to define this in PatternFly in the future.

@jeff-phillips-18
Copy link
Copy Markdown
Member

@kybaker

@color-pf-silver: #c0c0c0;

seems very close to either:
@color-pf-black-300: #d1d1d1;
or
@color-pf-black-400: #bbb;

Is it worth defining a new color variable?

@kybaker
Copy link
Copy Markdown
Contributor

kybaker commented Aug 30, 2016

@jeff-phillips-18 @priley86 We should avoid creating any new variables we have plenty of values to choose for this, please go with @color-pf-black-300: #d1d1d1;

@priley86 priley86 force-pushed the treeview-testing branch 2 times, most recently from 49ed86b to 82bba62 Compare August 30, 2016 19:34
@priley86
Copy link
Copy Markdown
Member Author

@jeff-phillips-18 @dlabrecq @bleathem i've updated this PR after publishing the V2 changes in our fork. Please give this one more look!

Comment thread src/js/patternfly.js Outdated
black700: '#4d5258',
black800: '#393f44',
black900: '#292e34',
black950: '#363636',
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need to add this? Why not just use the existing black900?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@jeff-phillips-18 I'm ok with this...just noticed nothing had been added to exactly match the body color #363636. updating...

@bleathem
Copy link
Copy Markdown
Member

This PR build and runs as expected. I see the changes on the treeview pages that we expect. The code changes seem reasonable to me.

👍

* incorporates V2 of treeview
* adds checkboxes and hierarchal checks tests
* adds highlight select and new highlight styles
* adds lazy load test
* adds prevent unselect test
* adds disabled nodes test
* ensure list-group-item overflow is handled
@jeff-phillips-18
Copy link
Copy Markdown
Member

LGTM

@jeff-phillips-18 jeff-phillips-18 merged commit 1fb756c into patternfly:master Aug 30, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants