fix: Unedited markdown widgets not persisting

[AkshatJawne]

Resolves #1777

Changes Implemented:

• Removed check to return null if the content is the default content (meaning it is unedited), which was resulting in the window disappearing on refresh

[mofojed]

We should never be calling hydrate from within a dehydration function.

Dehydration is taking a panel and outputting a serializable object such that we can serialize the layout and save it; hydration is the opposite, where we take a serialized layout, and hydrate it into objects (such as a grid model).

When we're dehydrating the layout, we check if the returned dehydration is null, and if it is, we omit the component from the layout completely:

web-client-ui/packages/dashboard/src/layout/LayoutUtils.ts

Line 340 in 6cf1240

const dehydratedComponent = dehydrateComponent(component, itemConfig);

In this case though, we do want to save the panel to the layout, we just don't want to save the actual content with it. I don't think there's a case where we would want to drop it completely from the layout (which returning null does).

Digging into it a bit ... I think you should actually remove this custom dehydrateMarkdown function completely, and just use the default hydrate/dehydrate functions (omitting the last two parameters from the call to registerComponent below). Then need to change MarkdownPanel.getClosedMarkdowns to filter out any panel that doesn't have a markdown set.

Actually found another bug too - if we return null from the dehydration function (as we're doing above), PanelManager throws an error:

Error while emitting event: TypeError: Cannot set properties of null (setting 'parentStackId')
    at PanelManager.addClosedPanel (PanelManager.ts:365:54)
    at PanelManager.handleClosed (PanelManager.ts:347:10)
    at EventHub.emit (EventEmitter.ts:69:22)

You can see this by just opening and closing a markdown panel before your changes. We should have a check there for if the dehydrated component is null, and if it is, skip pushing it onto the closed array.

[AkshatJawne]

Thanks for the information on hydration, it was very helpful! Will make the changes and will take a look at the other bug you mentioned

[mofojed]

We don't want to do this - right now this change is causing a regression, which is re-opening closed markdown widgets.

If you open a markdown panel, edit it to add some text, then close it, and then open a new markdown panel, you get a starting page prompting you to re-open one of the previously closed markdown panels or to just create a new one:

I think what you should do is just set null for content from here, then in MarkdownPanel is where it will actually decide whether to show the starting page or the default content (when it calls getClosedMarkdowns).

[AkshatJawne]

Just pushed changes, if I understood correctly when you said

then in MarkdownPanel is where it will actually decide whether to show the starting page or the default content (when it calls getClosedMarkdowns).

It now works such that on initial opening of a Markdown, you get this panel, given that when getClosedMarkdowns is called in MarkdownPanel, the condition for the starting page appearing is whether the state isStatePageShown is set to true:

However, after refreshing after editing, we see the default content:

[AkshatJawne]

Looking into e2e failures

[AkshatJawne]

Still failing on e2e with Mathjax, investigating why

[AkshatJawne]

@mofojed changed the getClosedMarkdowns, but the approving review has gone stale, so if you could approve it again after reviewing, that would be great!