Improvement oppurtunities #8
Description
Component Improvement Plan
Component updates
WidgetLayout.tsx
initialTemplate prop and internal state management
I would not manage the template state inside the component. Template should be mandatory, so should the onTemplateChange
documentationLink prop
This should not exist. If the consumer would like to customize the empty state they can use the emptyStateComponent. We can export the LayoutEmptyState component and provide an empty state customization interface there. If somebody needs to customize it, they can re-write the defaults. We want to reduce prop "bubbling" as much as possible.
analytics prop
This prop should not exist, it's too specific. What we should do is expose an "event interface" where we can have callbacks defined for specific events including but not limited to:
- on drag
- on widget add
- on widget remove
- etc...
The base set of events should be all places in which the analytics prop is referenced
drawerInstructionText prop
I'd say this should be the same as is with the empty state. Expose the drawer and allow customization of the component rather than passing props from the root.
WidgetDrawer.tsx
Inline styling
<div style={{ position: 'sticky', top: 0, zIndex: 100, display: 'flex', justifyContent: 'flex-end', padding: 'var(--pf-t--global--spacer--md)' }}>Enhance the interface to customize text and accessibility labels
For example:
aria-label="Close widget drawer"
Add widgets
Is hard coded. We should extend the accessibility coverage and provide a customization interface. This will be required for translated apps.
Unnecessary void functions
onDragStart={() => {}}
onDragEnd={() => {}}
GridLayout.tsx
widgetMapping distribution
This is a fairly stable prop and is used in almost all places. It should be shared via a simple "configuration" context within this module.
drop item template
const droppingItemTemplate: ReactGridLayoutProps['droppingItem'] = useMemo(() => {
if (currentDropInItem && isWidgetType(widgetMapping, currentDropInItem)) {
const widget = widgetMapping[currentDropInItem];
if (!widget) {return undefined;}
return {
...widget.defaults,
i: droppingElemId,
widgetType: currentDropInItem,
title: 'New title',
config: widget.config,
};
}
return undefined;
}, [currentDropInItem, widgetMapping]);The title: 'New title', is not customizable. It should be. A new widget title should be either provided within the widget mapping and have a configurable default value. It can also be provided via the customization context.
ExtendedLayoutItem type
Because of the widget mapping, we also have to make this type generic. The top level props item needs to have additional generic interface in itself so the type of individual items can be further customized. For example, HCC will require additional scope, module, and importName props on our widget mapping as we will use a single component for all widgets.
LayoutEmptyState interface
Needs to be exported in case customization of empty state is needed to avoid extensive prop passing.
Needs additional interface to customize the default text. Required for translations.
internal state handling
const [internalTemplate, setInternalTemplate] = useState<ExtendedTemplateConfig>(template);Should not exist, the template should be always passed by the package consumer.
inline css
style={{ position: 'relative' }}generic id
<div id="widget-layout-container" style={{ position: 'relative' }} ref={layoutRef}>This will either have to be configurable (config context) or have a random ID so it's not causing issues with duplicate IDs. PF does have a system to generate IDs.
resize handle customization
resizeHandles={['sw', 'nw', 'se', 'ne']}We may not always want these. Needs to be customizable.
GridTile.tsx
Header/Footer customization
Right now there is no way of customizing the header or footer component of a grid tile. We should split the component into these building blocks, expose them and provide an interface for full customization of the header component.
CSS Updates
CSS rules names
The current CSS class names are still using the HCC prefixes. We should move to the classic PF ones
.widg-c-drawer__header{}
// should be something like
.widg-pf-c-drawer__header{}
// consult PF team"Dangerous" rules
position: 'sticky'
Rules like sticky do have requirements on the parent component. We should test it with different containers and probably develop a wrapper component (default layout/page) that will have the necessary required rules which can then be used in the app.
Tests
We are missing both unit and "e2e" tests.