Draft SubscriptionManager compound component structure proposal

[mashikag]

In Apex's last technical hour, we agreed that it would be a good idea if the subscription manager was developed as a React compound component. The main reason being that we would like the component to be developed in a dedicated package, while having the data being provided by a user of this component, i.e. the component is dependent on data being provided via props.

We believe that developing the subscription manager as a compound component will help us to avoid unnecessary renders on data store changes.

[mashikag]

Draft Proposal

const SubscriptionSites = () => {
    const { siteSubs } = useSelector( getSiteSubscriptions );
    const dispatch = useDispatch();
    return (
        /* SiteSubscriptions container creates ctxt and is responsible for implementing search, filtering and sorting */
        <SubscriptionManager.SiteSubscriptions
            value={ siteSubs }
            defaultFilter={ {} }
            defaultSort={ { by: 'date', order: 'desc' } } >
            { siteSub => (
                <SubscriptionManager.SiteSubscriptions.Item
                    value={ siteSub }
                    onUnsubscribe={ () => dispatch( unsubscribeFromSite( siteSub ) ) }
                    onNewPostNotificationChange={ () => dispatch( toggleNewPostNotification( siteSub ) ) }
                    onNewPostEmailNotificationChange={ () => dispatch( toggleNewPostEmailNotification( siteSub ) ) }
                    onNewPostEmailFrequencyChange={ () => dispatch( toggleNewPostEmailFrequency( siteSub ) ) }
                    onNewCommentEmailNotificationChange={ () => dispatch( toggleNewCommentEmailNotification( siteSub ) ) }
                />
            )}
        </SubscriptionManager.Sites>
    );
}

const SubscriptionComments = () => {
    const { commentsSubs } = useSelector( getCommentsSubscriptions );
    const dispatch = useDispatch();
    return (
        /* CommentsSubscriptions container creates ctxt and is responsible for implementing search, filtering and sorting */
        <SubscriptionManager.CommentsSubscriptions
            value={ commentsSubs }
            defaultFilter={ {} }
            defaultSort={ { by: 'date', order: 'desc' } } >
            { ( commentsSub ) => (
                <SubscriptionManager.CommentsSubscriptions.Item
                    value={ commentsSub }
                    onUnsubscribe={ () => dispatch( unsubscribeFromComments( commentsSub ) ) }
                    onNewCommentsNotificationChange={ () => dispatch( toggleNewCommentsNotification( commentsSub ) ) }
                    onNewCommentsEmailNotificationChange={ () => dispatch( toggleNewCommentsEmailNotification( commentsSub ) ) }
                />
            ))}
        </SubscriptionManager.CommentsSubscriptions>
    );
}

const SubscriptionSettings = () => {
    const { userSettings } = useSelector( getUserSettings );
    const dispatch = useDispatch( );
    return (
        <SubscriptionManager.UserSettings
            value={userSettings}
            onSubmit={ value => dispatch( modifyUserSettings(value) ) }
        />
    );
}

// Use context for storing the current selected tab
const SubscriptionManagementPage = () => {
    const {
        sites: siteSubsTotal,
        comments: commentsSubsTotal
    } = useSelector( getSubscriptionTotals );
    return (
        <SubscriptionaManager>
            <SubscriptionManager.TabSwitcher
                tabs={ [
                    { id: 'sites', title: 'Sites', content: SubscriptionSites },
                    { id: 'comments', title: 'Comments', content: SubscriptionComments },
                    { id: 'settings', title: 'Settings', content: SubscriptionSettings }
                ] } />
        </SubscriptionaManager>
    );
}

Quick Component Breakdown

SubscriptionManager

SubscriptionManager.TabsSwitcher

SubscriptionManager.Sites SubscriptionManager.Sites.Item

SubscriptionManager.Comments SubscriptionManager.Comments.Item

SubscriptionManager.UserSettings

[TimBroddin]

Very nice design!

One thing I'm wondering is about the tabs. Shouldn't each tab be tied to a route, and only render its view when visiting said route? Maybe you already thought of this for the logic inside SubscriptionManager.Tabs, of course.

I did some quick search and there is a useCurrentRoute hook we could use inside client/components/route/index.js.

[mashikag]

One thing I'm wondering is about the tabs. Shouldn't each tab be tied to a route, and only render its view when visiting said route? Maybe you already thought of this for the logic inside SubscriptionManager.Tabs, of course.

This is actually what we were exploring with @phcp for the last 2 hrs. We were exploring different design patterns we could employ around it.

We thought that it would be ideal if we could use react-router for tab rooting. However, having react-router alongside pagejs seemed not too work. Route changes were handled by pagejs but they were not intercepted by BrowserRouter (from react-router). I did not investigate much, but it made sense to me that it may not work, having pagejs alongside.

Then we noticed how stepper, login and few other calypso single page apps which are built using their own entry point. They have their own path handler registered in expres.js server so that their own js file is served. That way, for example, they are able to apply react-router instead of pagejs for their own routing. I wish I was aware of this approach at the start of my spiking. At this stage, with the pressure to deliver, it feels like it would add too much overhead.

Therefore, I believe we will go for the old "good" pagejs routing. 😂 Thanks for mentioning the useCurrentRoute, I was thinking of getting the current path from the page context and passing it down as a prop on page render.

[mashikag]

Me and @phcp looked a bit more into it. We considered the following structure and had it half working:

const SubscriptionManagementPage = () => {
    return (
        <SubscriptionManager>
            <SubscriptionManager.Tabs>
                <SubscriptionManager.Tab path="/subscriptions/sites">Sites</SubscriptionManager.Tab>
                <SubscriptionManager.Tab path="/subscriptions/settings">Settings</SubscriptionManager.Tab>
            </SubscriptionManager.Tabs>

            <SubscriptionManager.Router>
                <SubscriptionManager.Route path="/subscriptions/sites">
                    Sites content here
                </SubscriptionManager.Route>
                <SubscriptionManager.Route path="/subscriptions/settings">
                    Settings content here
                </SubscriptionManager.Route>
            </SubscriptionManager.Router>
        </SubscriptionManager>
    );
};

Then we came across this React.Children documentation. It makes the following point:

It made us retrospect over our idea. We agreed to follow one of the suggested alternatives from that documentation (see the TabSwitcher example). It may be not elegant as react-router's structure, but it will be more maintainable.

[mashikag]

cc @Automattic/team-calypso-frameworks for awareness

Any feedback would be greatly appreciated. 🙌
I am free to provide more context on a call, or async, if needed. 🙇‍♂️

The issue is in relation to development of the pdDOJh-1t8-p2.