feat(@clayui/drop-down): LPD-47559 Cascading Menu should turn into dr…

[pat270]

…illdown in smaller screens

https://liferay.atlassian.net/browse/LPD-47559

I was running into errors returning drilldown when resizing. This PR renders Drilldown if the window width is < 768px and cascade at larger sizes. I also couldn't figure out how to add dropdown-menu-indicator-start on dropdown-menu based on if an item had an icon on the left side in Drilldown.

[ethib137]

Thanks @pat270 ... so what still needs to be fixed on this one? The screenshot shows the icon overlapping the text of an item, but what else is still not working?

[pat270]

@ethib137 I think I figured out a way to get the icon to work. The other thing that's not working, if needed, is that it doesn't convert to drilldown or cascading menu when resizing the window. It just renders drilldown or cascading menu depending on the starting window width.

[matuzalemsteles]

I believe this is a more complex task than it seems, the DropDownWithItems component has more features than DropDownWithDrilldown, so it wouldn't be just a matter of mapping the items to the Drilldown data structure, it would be necessary to refactor the component and add support for all types, ideally making it so that both components can reuse code to avoid duplication and complexity.

So this requires more work here and going with this may break the DropDownWithItems component when rendered on a smaller screen. Another strategy would be to implement drilldown only for the case when the type is contextual.

[matuzalemsteles]

You can use our useIsMobileDevice hook in @clayui/shared for this. One thing to note is that when the user resizes the screen this value does not change so we would have to add an event to update this value whenever it is resized. We can do this in the hook with a throttle if necessary.

[ilzamcmed]

Since @pat270 is on PTO, I am working on this fix

[ilzamcmed]

@matuzalemsteles, I've implemented the fix, but I ran into some issues due to the way hooks render. Since hooks can't be called inside an if statement and useId() relies on useMemo, I worked around it by adding a separate ID specifically for the drilldown.

Do you have any suggestions for a different approach?

Thanks!

[matuzalemsteles]

@ilzamcmed I'll take a look at this. The ideal is to have a unique ID per instance because we can have several components rendered on the page and this will have the same ID.

[matuzalemsteles]

I'm going to work on it now, we need to rewrite some parts of the component, ideally we should refactor both components but this can take a long time so I'll try to add some new rules to the cascading menu to have the same behavior as the drilldown.

[ilzamcmed]

Thanks @matuzalemsteles!
Let me know if I can help on anything.

[matuzalemsteles]

Well, commit refactoring the Drilldown and DropDownWithItems components, now both components render the same items for compatibility and also support almost the same props. It is now safe to use Drilldown in DropDownWithItems when on a mobile version.