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

Ability added to use custom icons along with default icons in Node Le… #451

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rajputvarun591
Copy link

@rajputvarun591 rajputvarun591 commented Feb 10, 2023

…ading.

Description

Related Issues

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I signed the CLA.
  • All tests from running flutter test pass.
  • flutter analyze does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

@jamesderlin
Copy link
Collaborator

Please see my comment on #447. Do not keep filing new PRs and closing them if you encounter a problem.

@rajputvarun591
Copy link
Author

@jamesderlin Sorry for that bro. this is finally I have created with no issues. I will delete rest of the previous.

@rajputvarun591
Copy link
Author

@jamesderlin I am very sorry to do that, actually it was my first time to make a PR on any of the plugin repository. I just was curious about it. Kindly delete the unwanted PR if you can or just decline them. Thank you for your support!

@rajputvarun591
Copy link
Author

@jamesderlin And why so many PR's ? aren't you guys taking the changes from other dev? By when my PR will be merged and reflect the changes on pub.dev ? any idea about this?

@jamesderlin
Copy link
Collaborator

If you mean why are there so many unmerged PRs, this repository is the home to multiple packages, each owned by different individuals. Those individuals are responsible for maintaining their packages and for merging PRs. Some of those individuals might have moved on to other projects.

@jamesderlin jamesderlin requested a review from polina-c February 11, 2023 04:09
@jamesderlin jamesderlin added the p: flutter_simple_treeview Related to package:flutter_simple_treeview label Feb 11, 2023
@rajputvarun591
Copy link
Author

@jamesderlin Thanks for your quick response! I will contribute more features in the other packages too. and if it will be possible I will open the old PR by update code there, so that old PR's also been justified. Thanks for support.

@jamesderlin
Copy link
Collaborator

@jamesderlin Thanks for your quick response! I will contribute more features in the other packages too. and if it will be possible I will open the old PR by update code there, so that old PR's also been justified. Thanks for support.

That would just be confusing. Just file new PRs for unrelated PRs from now on.

this.treeController})
: nodes = copyTreeNodes(nodes),
/// This widget will be takes place of default icon when Node will not be in expanded state
final Widget? primaryIcon;
Copy link
Member

Choose a reason for hiding this comment

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

Rename to iconExpanded

final Widget? primaryIcon;

/// This widget will be takes place of default icon when Node will be in expanded state
final Widget? secondaryIcon;
Copy link
Member

Choose a reason for hiding this comment

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

Rename to iconCollapsed

final Widget? primaryIcon;

/// This widget will be takes place of default icon when Node will be in expanded state
final Widget? secondaryIcon;
Copy link
Member

Choose a reason for hiding this comment

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

Here and in another one, make widget non-nullable and assign default value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p: flutter_simple_treeview Related to package:flutter_simple_treeview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants