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

DockviewApi.toJSON doesn't include panel titles #258

Closed
alakhpc opened this issue May 29, 2023 · 12 comments · Fixed by #260 or #265
Closed

DockviewApi.toJSON doesn't include panel titles #258

alakhpc opened this issue May 29, 2023 · 12 comments · Fixed by #260 or #265
Labels
bug Something isn't working

Comments

@alakhpc
Copy link

alakhpc commented May 29, 2023

Hi, I absolutely love the work you're doing with this library :), and am trying to use it.

Describe the bug
I think I've run into a bug where the panel title is not included in the JSON, causing the title to reset to the id when you refresh (persistent layout).

Steps to reproduce the behavior:

  1. Create a panel with a title, the panel renders with the correct title.
  2. Check DockviewApi.toJSON()
  3. See blank title

Expected behavior
Expected title to be included in the JSON

{
   "id":"xlfqnma9wioq948mmf6mh7u2",
   "contentComponent":"heatmap",
   "params":{
      "title":"Heatmap"
   },
   "title":""
}
@mathuo
Copy link
Owner

mathuo commented May 29, 2023

Thank you and thanks for the report. Which version are you using?

I have an example here using the latest version 1.7.2 which seems to persist and reload a custom panel title but I may be missing something. You might be able to adjust my example to "break it"?

https://codesandbox.io/p/sandbox/github/mathuo/dockview/tree/master/packages/docs/sandboxes/layout-dockview

@alakhpc
Copy link
Author

alakhpc commented May 30, 2023

I think I've narrowed it down.
It seems to happen only when I have a title, and a prop called title.

  api.addPanel({
    id: "panel_1",
    component: "default",
    // title here 
    title: "Heatmap",
    params: {
      // title here also
      title: "Heatmap",
    },
  });

Reproducible codesandbox:
view link: https://lnz49u-3000.csb.app/

https://codesandbox.io/p/sandbox/stoic-napier-lnz49u?file=%2Fsrc%2Fapp.tsx%3A26%2C1-33%2C6

  1. Click reset Layout, title of panel_1 should be Heatmap
  2. Refresh, title changes to panel_1
Screen.Recording.2023-05-30.at.12.27.09.PM.mov

@mathuo
Copy link
Owner

mathuo commented May 31, 2023

Nice! Thanks, found the problem.

There is a bug in the code where the title parameter and a key of title in the user parameters are used wrongly, I believe I've fixed with the attached branch. I will confirm and get a patched release out.

@mathuo mathuo added the bug Something isn't working label May 31, 2023
@mathuo
Copy link
Owner

mathuo commented May 31, 2023

Sandbox example using the build created from this branch as evidence of a working fix.

https://codesandbox.io/s/layout-dockview-forked-2nfmmq

@alakhpc
Copy link
Author

alakhpc commented May 31, 2023

That seems great!

@mathuo mathuo reopened this Jun 3, 2023
@mathuo
Copy link
Owner

mathuo commented Jun 4, 2023

1.7.3 published with fix - thanks for the help in finding the bug

@mathuo mathuo closed this as completed Jun 4, 2023
@alakhpc
Copy link
Author

alakhpc commented Jun 5, 2023

I think I've discovered another related bug:

If I use api.updateParameters inside a panel, it resets the title, even though the original title was not in the params

@mathuo mathuo reopened this Jun 5, 2023
@mathuo
Copy link
Owner

mathuo commented Jun 5, 2023

Damn, sorry yeah I see this - seems to be an existing bug again. I will take a deeper look at this logic and add tests.

@mathuo mathuo reopened this Jun 6, 2023
@mathuo
Copy link
Owner

mathuo commented Jun 6, 2023

Sandbox example using a copy that has these changes #265 here.

Shows examples of both api.setTitle and api.updateParameters being used and covering the case where somebody may name a custom parameter title.

These changes are merged to master and will aim to release if all continues to look good.

@alakhpc
Copy link
Author

alakhpc commented Jun 7, 2023

I think I'm still seeing the bug, gonna try and make a sandbox

@alakhpc
Copy link
Author

alakhpc commented Jun 7, 2023

My bad, just had to clean node_modules.

Looks good, Thanks!

@mathuo
Copy link
Owner

mathuo commented Jun 11, 2023

v.1.7.5 published with fixes

@mathuo mathuo closed this as completed Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
2 participants