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

Create feature discovery component #1068

Open
9 tasks
frattaro opened this issue Dec 3, 2021 · 1 comment
Open
9 tasks

Create feature discovery component #1068

frattaro opened this issue Dec 3, 2021 · 1 comment
Assignees

Comments

@frattaro
Copy link
Contributor

frattaro commented Dec 3, 2021

Is your feature request related to a problem? Please describe.

As a developer, I need a feature discovery component.

Describe the solution you'd like

Supplying a js object configuration to an AFeatureDiscovery component, the UI should display a series of popups over an overlay. If any configuration step targets an element on the page, the overlay should appear to be cut out around that element, and any AMenu can target the cut-out area. Since targeting an element is optional, it'll also need to take advantage of ADialog. Question: should APopOver be used instead of AMenu? It is expected that they have buttons after all. Is there a way to do it so the element is still interactable?

Since it can't be expected that developers can pass around deeply nested refs (unless they're supplied with a context... !?), I believe the configuration step will need to use a selector for identification. That invites the use case where the selector targets more than one element. Question: use querySelectorAll and have the cut-out area encompass them all, or use querySelector and only target the first element?

One aspect to consider is menu placement. Should it compare the absolute position of the target in the window to attempt to display the menu towards the center of the screen? Maybe as an enhancement, but certainly could give the developer the option of choosing the placement of the menu.

Each menu/dialog should have previous/next buttons and a feature that allows the user to escape feature discovery.

I good amount of accessibility considerations should be covered by ADialog and AMenu, but worth adding keyboard navigation: arrow keys, escape key.

  • overlay with cut-out
  • supports keyboard navigation
  • supports dialogs
  • supports menus
  • supports in-dialog/menu navigation
  • is documented
  • has tests with appropriate coverage
  • placement
  • scrolling targets into view

Additional context

If we did go the context route it would be optionally separately declarable but worth rolling into AApp, could do something like:

const buttonRef = useRef(null);
const {addFeature, runSet} = useFeatureDiscovery();

addFeature({
  setId: "modulesFeatureDiscovery", // what cycle gets this step applied?
  step: 1.5, // float
  target: buttonRef, // optional
  content: (someUsefulPropsMaybe) => (
    <p>
       Can put anything we want in here.
       <ATextInput />
     </p>
  ),
  placement: "bottom-left" // optional
});

runSet("modulesFeatureDiscovery")

this would be instead of supplying the full configuration in one spot, and using css selectors instead of refs. But would it be less maintainable this way?

Sources of Inspiration:
react-joyride demo site

material design feature discovery documentation - radically different from what we're doing but it's good inspiration

@guptar8jan guptar8jan self-assigned this Dec 3, 2021
@brennarvo
Copy link
Collaborator

When it comes to deeply nested refs, I recommend a pattern used by some accessibility-first component libraries. Like you suggested, there is a context in which child components "register" their DOM nodes with a callback ref. This also enforces some good component composition because AFeatureDiscovery would have child components to represent each step. Maybe something like:

<AFeatureDiscovery>
    {/* AFeatureDiscovery.Step registers its DOM node (p tag) in the AFeatureDisovery parent context */}
    <AFeatureDiscovery.Step>
        <p> {/* I'm index 0 in AFeatureDiscovery's context with a reference to my node */}
            Step one
            <ATextInput />
        </p>
    </AFeatureDiscovery.Step>

    <AFeatureDiscovery.Step placement='bottom-left'>
        <p> {/* I'm index 1 in AFeatureDiscovery's context with a reference to my node */}
            Step two
            <ATextInput />
        </p>
    </AFeatureDiscovery.Step>
</AFeatureDiscovery>

This pattern also makes things like arrow key navigation quite easy since the child components that are registered can almost be thought of as a linked-list in the context in which they are registered. So when you press arrow-forward or arrow-back while on a component, it's easy to grab and use the most adjacent (either left or right) DOM node (useful for programmatic focus, tabIndex manipulation, etc.)


For reference on the "descendants" pattern I mentioned:

0: https://github.com/chakra-ui/chakra-ui/blob/main/packages/descendant/README.md
1: https://github.com/reach/reach-ui/blob/develop/packages/descendants/README.md

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

No branches or pull requests

3 participants