writer / writer-framework

No-code in the front, Python in the back. An open-source framework for creating data apps.
https://dev.writer.com/framework/introduction
Apache License 2.0
1.32k stars 75 forks source link

Custom Sidebar is not put in the sidebarContainer slot #217

Open ZBMO opened 9 months ago

ZBMO commented 9 months ago

functions for :component-filter check if the c.type == 'sidebar' or c.type != 'sidebar'.
The result is that a custom sidebar isn't placed in the appropriate slot (having its type prepended with 'custom_') and is arranged on the page the other layout components.

a fix would be to change the equality checks on lines 6 and 19 of CorePage.vue to use .includes() instead of ==. I would like to make a PR for this

image

ramedina86 commented 9 months ago

Good find, thanks for sharing! I didn't contemplate people might want to develop their own sidebars. This is a limitation that shouldn't exist.

To be honest, the mechanism is a bit hacky as it is, and I think partially matching the string "sidebar" would be even hackier.

I'd prefer having something that tells the Page where to put it, whether it's the core Sidebar or a custom component. I'm thinking an additional attribute in the StreamsyncComponentDefinition (in streamsyncTypes.ts)...

export type StreamsyncComponentDefinition = {
    [...]
    insertionArea?: "default" | "sidebar";
};

I believe this is cleaner and would allow us to develop e.g. "footer" or "topbar" in the future, without breaking anything.

Feel free to send a PR; it'd be appreciated. Should still be quite straightforward I believe. Apart from that change in type as shown above, I think it'd just be checking whether the component is c.insertionArea == "sidebar" or not in CorePage.vue. Please let me know your thoughts.

ramedina86 commented 9 months ago

Hi @ZBMO , can you please confirm whether you want to work on this PR? Otherwise we're happy to take it ourselves

ZBMO commented 9 months ago

@ramedina86 sorry for the delay, I'd like to but I won't be able to work on it till next week. I understand if you need it done sooner.

ramedina86 commented 9 months ago

@ZBMO no worries at all, next week is fine. And of course no pressure, just want to avoid two people working on it at the same time.

ramedina86 commented 8 months ago

@raaymax please take care of this which will also help address the complexities around reusing positionless components