Complete action items from grommet-leaflet-core review

[taysea]

Based on initial review of grommet-leaflet-core, the following action items remain:

• rename grommet-leaflet-core to grommet-leaflet (Rename grommet-leaflet-core to grommet-leaflet #42)
• Ideate on way to allow caller to override default Popup pad: Complete action items from grommet-leaflet-core review #33 (comment)
• Assess screen reader behavior with Map and markers. Come up with best practices for what should be read out by a screen reader and determine if anything can be built into grommet-leaflet-core or have an example demonstrating best practice: Accessibility summary/notes
• Think about how popup should be applied to an individual Marker. Child of marker popup is the native behavior, but that’s counter to the grommet pattern where children would alter the content within a Marker. (See Fix hooks render error #38)
• Cluster theming, specifications for kind + size overrides, think about how that gets built up Kind should override size in cluster theme #43
• Double check that zIndex in .css file is working for base theme + hpe theme

Deliverables might include:

• PRs addressing changes
• Summary notes about accessibility behaviors/decisions regarding API structure

[jcfilben]

Accessibility notes: (tested in voiceover)

• When focus is moved to the map container the screenreader reads out all the clusters/pins that are on the map. There is no indication that the user is on a map container.
• Clicking on a cluster/pin opens the popup but doesn't give any indication that anything happened.
• After a popup is opened the user must navigate through all the clusters/pins on the map to get to the popup content. There is no way to associate a popup with a particular pin/cluster.
• After interacting with the zoom controls with ctrl+option+space focus is unexpectedly moved back to the map container.
• After interacting with controls popups can no longer be opened (occasionally I am able to get one to open but the majority of the time they will not).

Leaflet's accessibility guide: https://leafletjs.com/examples/accessibility/

Taylor's additions:

Reading through this and given some limitations may be on the leaflet side. My suggestions would be:

• Map should be a secondary data presentation, but a fallback to a tabular or list view should be available as well
• Should we provide a clear a11yTitle to a marker/cluster that captures anything that would be in a popup?

[jcfilben]

Cluster theming:
Size should be specified outside of kind. This allows for generic sizes that can be used across kinds. Any props defined in a kind will take precedence over the size props. This structure gives us flexibility if down the road we need to support size within kind.

Theme structure:

cluster: {
  default: {
    // this is a kind of cluster
    container: {
      // any box props, will take precedence over box props defined in size
    }
  }
  size: {
    small: {
      container: {
        // any box props
      }
      label: {
        // any text props
      }
    }
  }
}

[taysea]

Capturing a discussion Jessica and I had offline re: theme structure: Should kind objects be nested under another level called kind?

This is different than grommet theme.button. However, the rationale would be that a top-level theme object would refer to a property on a component. Then, the nested objects would align with accepted values for that property.

For example:

cluster: {
   kind: {
      critical: ...
   },
...
}


<Cluster kind="critical" />

Not a strong opinion one way, but wanted to voice it before we publish and commit to a theme structure.

[taysea]

Customizing popup pad and other leaflet popup props

After digging into this, Jessica and I realized that beyond allowing the caller to pass custom box props to their Popup container, we should also support the ability for callers to pass accepted options for leaflet's popup (in terms of grommet-leaflet truly being an extension of leaflet and not to opinionated).

Some ideas we had were to:

1. Embed react-leaflet's "Popup" inside our "Popup" component. Then, similar to react-leaflet's implementation, callers would need to wrap their popup content in a Popup wrapper that accepted any react-leaflet Popup props plus boxProps which would style the Popup container.

Popup.jsx

const Popup = ({ boxProps, children ...rest}) => {
   return (
      <LeafletPopup {...rest}>
         <Box {...theme.popup} {...boxProps}>
            {children}
         </Box>
     </LeafletPopup>
}


Then the caller's implementation would be something like


<Marker>
   <Popup autoClose={false} boxProps={{ pad: 'medium' }}>
      <MyPopupContent>
   </Popup>
</Marker>

This approach worked for the Markers but did not work for the Clusters which are using ReactDOMServer.renderToString. An error that LeafletPopup can't be used outside of MapContainer is thrown.

So we will need to think on other approaches and what the API surface should like.

[jcfilben]

I created two follow on tickets for the accessibility and popup work.
#44
#45