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

Adding group doesn't allow for group creation options #465

Closed
NaNgets opened this issue Jan 21, 2024 · 4 comments · Fixed by #468
Closed

Adding group doesn't allow for group creation options #465

NaNgets opened this issue Jan 21, 2024 · 4 comments · Fixed by #468
Milestone

Comments

@NaNgets
Copy link
Contributor

NaNgets commented Jan 21, 2024

Is your feature request related to a problem? Please describe.
In the dockviewComponent there is an expose addGroup function which only accepts position options, it uses an internal non-exposed createGroup function which DOES receive group options but the actual call doesn't call with any options (anywhere except when deserializing from JSON).

Describe the solution you'd like
addGroup should either accept a group with position options or a group options to send to the createGroup.

@mathuo
Copy link
Owner

mathuo commented Jan 21, 2024

Could you put it on a branch clones from the main repo rather than a fork? Then I can link it up to this issue too.

Happy to add this in, perhaps it'll be better though to merge the two options into one object for the addGroup method. e.g.

addGroup(options?: AddGroupOptions & AddGroupOptions): DockviewGroupPanel

Perhaps even better just to extend the AddGroupOptions type since it's only used for this method anyway,

export type AddGroupOptions = (
    | AddGroupOptionsWithGroup
    | AddGroupOptionsWithPanel
    | AbsolutePosition
) &
    GroupOptions;

@NaNgets
Copy link
Contributor Author

NaNgets commented Jan 21, 2024

Hi @mathuo , for me to understand - you wanted me to clone this repo to my computer, create a branch, redo the changes, commit and push the branch straight into here, correct?

If so, I tried and received an error that I don't have permissions.

I'm not using github in my day-to-day so I don't completely understand how it works, feel free to correct me, but as far as I understand - creating a pull request from a fork into the original repo is fine and can also be linked the same way.

If you would like me to do something else I'd appreciate a bit of guidance or a link to a documentation on what to do. Thanks.

@mathuo
Copy link
Owner

mathuo commented Jan 21, 2024

Ah - actually yeah could you create a PR from your branch intto git fetch origin 465-adding-group-doesnt-allow-for-group-creation-options rather than master, that'll make it easier

@NaNgets
Copy link
Contributor Author

NaNgets commented Jan 21, 2024

Sure, I changed the merge target in #466 , now it should be fine.

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

Successfully merging a pull request may close this issue.

2 participants