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

Add example solidjs-with-modal-manager #26

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

noveogroup-amorgunov
Copy link
Contributor

@noveogroup-amorgunov noveogroup-amorgunov commented Jul 28, 2024

Hello, I made a new example of how to work with the modal manager. More information in the README.md . See live demo in https://stackblitz.com/github/noveogroup-amorgunov/feature-sliced--examples/tree/feature/add-example-solidjs-with-modal-manager/examples/solidjs-with-modal-manager?file=README.md

@illright
Copy link
Member

illright commented Aug 2, 2024

Hello, thanks for that! This is an interesting one from the architectural perspective. Ideally, layers at the bottom (in this case, Shared) should know nothing about layers at the top. In your example, you have DI, but the Shared layer still knows that there is a Login modal that will be registered and used. I like modal managers too, but I prefer them to be scoped. For instance, on my work project, we have a hook like this:

// pages/editor/ui/usePanel.ts
import { create } from "zustand";
import { devtools } from "zustand/middleware";

type PanelName = "sections" | "smartfields" | "search";

interface OpenedPanel {
  name: PanelName;
  anchor?: string;
}

interface PanelStore {
  openedPanels: OpenedPanel[];
  openPanel: (name: PanelName, anchor?: string) => void;
  closePanel: (name: PanelName) => void;
}

const usePanelStore = create<PanelStore>()(
  /* implementation snipped for brevity */
);

export function usePanel(name: PanelName) {
  const { openedPanels, openPanel, closePanel } = usePanelStore();

  const thisPanel = openedPanels.find((panel) => panel.name === name);

  return {
    isOpen: !!thisPanel,
    anchor: thisPanel?.anchor,
    open: (anchor?: string) => openPanel(name, anchor),
    close: () => closePanel(name),
    toggle: () => (thisPanel ? closePanel(name) : openPanel(name)),
  };
}

This hook is generic enough that most of its implementation can be placed in shared as a "store factory", but the actual modal names are only known by the page itself. What do you think about this?

Also I'd prefer if the slice session/login wasn't grouped. There's only one slice in that group, and slice groups are already an issue with people where they misunderstand the isolation rules of slice groups. And besides, does it really need to be a feature? I think this code should be in the home page.

I think the "Connection error. Try again later" message is confusing. I thought something was wrong with my setup and was about to comment about it, and then I realized that that is how it's supposed to be.

Curious to hear your thoughts

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 this pull request may close these issues.

2 participants