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

V2 changes to bootstrap-treeview#1

Merged
bleathem merged 63 commits intomasterfrom
patternfly
Aug 30, 2016
Merged

V2 changes to bootstrap-treeview#1
bleathem merged 63 commits intomasterfrom
patternfly

Conversation

@priley86
Copy link
Copy Markdown
Member

@priley86 priley86 commented Jul 21, 2016

  • Adds commits currently in the bootstrap-treeview develop branch
  • Adds #266, #263, and #262
  • Adds changes to span.image styling to resemble span.font styling and .gitignore changes. 47d9b72
  • Adds hierarchical checking of checkboxes b26e546
  • Adds fix for jquery 3, bc19d2c

@priley86 priley86 force-pushed the patternfly branch 2 times, most recently from c6aab84 to 3b9ebcb Compare July 21, 2016 21:50
@priley86
Copy link
Copy Markdown
Member Author

@bleathem @andresgalante @skateman @dlabrecq

I've attempted to capture all patternfly desired changes to bootstrap-treeview in this PR in our new fork. Since this project is no longer actively maintained, a new fork is needed.

Please let me know what you guys think.

@dlabrecq
Copy link
Copy Markdown
Member

Is this a POC? Looking through Gitter and obviously missed some conversation, so please fill me in.

Are you suggesting that the delevop branch is no longer maintained, too?

I recall Leslie was going to identify required features for a new component. (We should include her in this review.) Does this have all the features Patternfly is looking for?

@skateman
Copy link
Copy Markdown
Member

These features are filed as PR against the upstream repo ...

@priley86
Copy link
Copy Markdown
Member Author

@skateman @dlabrecq we are discussing the option of sourcing this fork in patternfly for the time being. The upstream maintainer has not been responsive and it seems we have a lot of contributions we'd like added very soon.

There will be more discussion on this before it's merged to Patternfly :)

@priley86
Copy link
Copy Markdown
Member Author

@bleathem @dlabrecq @skateman any other editions we'd like added now to this?

Once this is merged, we can add a new npm/bower module for this, "patternfly-bootstrap-treeview". We can always release and append new changes in our new fork later, in a timely manner :)

Afterwards, I will add a new PR to reference this in Patternfly.

@skateman
Copy link
Copy Markdown
Member

LGTM 👍

@priley86 priley86 force-pushed the patternfly branch 3 times, most recently from 46c6458 to d2bb044 Compare August 1, 2016 15:44
@priley86
Copy link
Copy Markdown
Member Author

priley86 commented Aug 5, 2016

@dlabrecq @skateman i've added a jquery v3 fix (we can use JSON.parse instead of now deprecated jQuery.parseJSON). I think this PR is ready for merge so we can adopt patternfly-bootstrap-treeview v2 (changes here) next release in Patternfly . I will follow up with another PR in Patternfly with changes noted here

@priley86
Copy link
Copy Markdown
Member Author

priley86 commented Aug 5, 2016

@skateman can you add a PR in the now forked upstream ?

@dlabrecq
Copy link
Copy Markdown
Member

dlabrecq commented Aug 9, 2016

LGTM. Although, I'm leary of merging with Brian and Andres on PTO. Don't want to be put on trial, again.

@priley86
Copy link
Copy Markdown
Member Author

priley86 commented Aug 9, 2016

@dlabrecq should be fine to hold off on this until @bleathem & @andresgalante review. We are still awaiting icons/design on the checkmarks anyhow. @skateman requested this be released sometime this month for mIQ, so will shoot for the following release (Aug. 30).

@priley86 priley86 force-pushed the patternfly branch 2 times, most recently from 75ffda2 to 58b84f9 Compare August 16, 2016 15:33
@priley86 priley86 force-pushed the patternfly branch 2 times, most recently from f594ffd to 8de91f2 Compare August 24, 2016 20:10
@priley86 priley86 changed the title Patternfly changes to bootstrap-treeview V2 Patternfly changes to bootstrap-treeview Aug 25, 2016
@priley86 priley86 changed the title V2 Patternfly changes to bootstrap-treeview V2 changes to bootstrap-treeview Aug 25, 2016
- Split render method, with renderNode added to render an individual node
- Resolved test issues with expand collapse functions
- The class node-selected is added via setSelectedState
- Removed inline style that sets color + background-color based on selected state
- Updated injected css to use options selectedColor + selectedBackColor
- Allows of user override via .css resource
jonmiles and others added 21 commits August 26, 2016 10:17
If enabled, the disabled node will keep its expanded/checked/selected state.

(cherry picked from commit d4ef058)
By setting `highlightChanges` to true, the changed checkboxes will have `changedNodeColor`.
The default checkbox state can be reset by calling the `unmarkCheckboxChanges()` function.

(cherry picked from commit 798f7e8)
@priley86
Copy link
Copy Markdown
Member Author

@dlabrecq @skateman @bleathem please check once more when you get a chance. I think we are ready.

@skateman
Copy link
Copy Markdown
Member

LGTM 👍

@andresgalante
Copy link
Copy Markdown
Member

@priley86 I really don't know what I should review 😄 sorry. If @skateman @dlabrecq and/or @bleathem are ok with it, then 🚀 !

@priley86
Copy link
Copy Markdown
Member Author

@andresgalante we are just reviewing @skateman's V2 changes before we incorporate in our fork. Would like to merge this one first, and then update #437 to reference V2 after it's published.

@dlabrecq
Copy link
Copy Markdown
Member

Not certain what's changed since I last reviewed, but looks ok? I'll defer to @bleathem .

@priley86
Copy link
Copy Markdown
Member Author

@bleathem i think this is ready for merge. can you do the honors?

@bleathem
Copy link
Copy Markdown
Member

This PR has been reviewed for functional and visual correctness using the patternfly test pages by both designers and developers and deemed to be acceptable. A scan of the code changes looks reasonable, and I'm happy to see the supporting documentation changes.

I'd say this PR is ready to merge.

@bleathem bleathem merged commit bcf605e into 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.

6 participants