Fix Size of SelectMenu in SelectWidget.

[victorchrollo14]

Fixes #4058.

Fixes the Size of Select Menu in SelectWidget, whose height was greater than the height of parent modal as a result users had to scroll both the Select menu as well the main modal to view all the field choices.

The Issue is fixed by passing a menuPortalTarget prop into the Select Menu, that renders the select menu directly in document.body rather than the parent Modal as a result when the height of select menu is greater than the parent modal the users won't have to scroll the select menu as well as the parent modal to view all the field choices.

Screencast.from.08-03-24.05.51.57.PM.IST.webm

[netlify]

✅ Deploy Preview for plone-components canceled.

Name	Link
🔨 Latest commit	2caf15a
🔍 Latest deploy log	https://app.netlify.com/sites/plone-components/deploys/661f53f65f93bc00084edd7d

[netlify]

✅ Deploy Preview for volto canceled.

Name	Link
🔨 Latest commit	2caf15a
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/661f53f6de91c1000831224b

[stevepiercy]

This PR needs review from a maintainer. I only reviewed the change log entry.

[stevepiercy]

Change logs should avoid implementation details, as the code covers that.

Suggested change

	Fix the size of Select Menu in SelectWidget to prevent users from having to scroll both the main modal and dropdown menu to view the field choices by adding a menuPortalTarget prop. @victorchrollo14
	In a modal with a `SelectWidget`, the parent modal no longer requires scrolling to view the choices in the select's options list. @victorchrollo14

[victorchrollo14]

Ok. Thank you.
Shall I wait for the maintainers to review and make all the suggested changes in a single commit.

[ichim-david]

@victorchrollo14 this is not a good fix, it looks bad to show the select options below the modal which has the grey overlay to bring focus only to the modal content.

I would do the following:

1. set overflow: hidden instead of auto
2. give a max height for the react-select__menu added in this modal to avoid
   going over the content area as just having overflow hidden means that URL
   is hidden as seen in my screenshot

EDIT:
You could try also without any overflow set on that content parent as hidden might hide something down the line or in responsive mode

[victorchrollo14]

@ichim-david.
Adding overflow: hidden to the parent component disable the user from seeing all the content in the modal.

Initially I tried adding a maxMenuHeight prop to select component which fixes the issue in /controlpanel/dexterity-types/, but it doesn't work in all the places for instances you can check out /controlpanel/user where the issue persists.

Screencast.from.10-03-24.06.39.56.PM.IST.webm

I do agree that showing the select options below the modal looks bad.
The select option is shown below the modal in several places like /controlpanel/date-and-time/ , /controlpanel/site and /controlpanel/language etc.

What do you think would be better to use, should we go with the menuTargetPortal or use menuMaxHeight prop.

[ichim-david]

@ichim-david. Adding overflow: hidden to the parent component disable the user from seeing all the content in the modal.

Initially I tried adding a maxMenuHeight prop to select component which fixes the issue in /controlpanel/dexterity-types/, but it doesn't work in all the places for instances you can check out /controlpanel/user where the issue persists.
Screencast.from.10-03-24.06.39.56.PM.IST.webm

I do agree that showing the select options below the modal looks bad. The select option is shown below the modal in several places like /controlpanel/date-and-time/ , /controlpanel/site and /controlpanel/language etc.

What do you think would be better to use, should we go with the menuTargetPortal or use menuMaxHeight prop.

@victorchrollo14 I appreciate the fact that you've done research and mentioned other places where we have these react-select input and how it would look, well done, it shows a good level of commitment to thinking of a fix that would be acceptable for more than one place.

That menu does not look good if it goes outside of the content area as it is and is for sure no no in a modal.

What is acceptable to go over the content area is the popup component that is used in the Contents section
where it has some shadows and the icon that triggers it is telling you that you are about to open a menu.

I would rather have the extra scrolling rather than this look as at least that is all enclosed in the section that contains these elements and in the case of add user I find it acceptable to use scrolling to get to the options I need.

We had some changes on the contents panel where a designer added some bottom breathing area for situations
where you would open the popup and it was later removed as a regression because it didn't look right for the
situation where you didn't open the menu.

You can continue to experiment if you like but take into consideration that again it has to look good in order
for the change to be accepted.

[victorchrollo14]

hey @ichim-david . I went through most of the pages where the SelectWidget is being used. I added a maxHeightMenu prop that would keep the Select menu within the modal. This is what I found out.

1. It fixes the issue in routes /controlpanel/dexterity-types/*/schema along with a similar issue in the homepage, when we click on personal tools -> preferences -> and then try selecting a language.
2. Doesn't cause issues in other places where SelectWidget is used. at least in all the places I looked into. ( I went through all the routes, not sure If I missed any)
3. There are a few exceptions, where although we add a maxMenuHeight prop, The select menu height doesn't change, example /controlpanel/date-and-time, /controlpanel/language etc.

What do you think? Would this be fine or should we try some other alternative solutions for this problem.

Screencast.from.11-03-24.10.09.34.PM.IST.webm

Screencast.from.11-03-24.10.02.37.PM.IST.webm

[ichim-david]

hey @ichim-david . I went through most of the pages where the SelectWidget is being used. I added a maxHeightMenu prop that would keep the Select menu within the modal. This is what I found out.

1. It fixes the issue in  routes `/controlpanel/dexterity-types/*/schema` along with a similar issue in the homepage, when we click on personal tools -> preferences -> and then try selecting a language.

2. Doesn't cause issues in other places where SelectWidget is used. at least in all the places I looked into. ( I went through all the routes, not sure If I missed any)

3. There are a few exceptions, where although we add a `maxMenuHeight` prop, The select menu height doesn't change, example `/controlpanel/date-and-time`, `/controlpanel/language` etc.

What do you think? Would this be fine or should we try some other alternative solutions for this problem.

Screencast.from.11-03-24.10.09.34.PM.IST.webm
Screencast.from.11-03-24.10.02.37.PM.IST.webm

@victorchrollo14 I don't see the maxHeightMenu prop after your latest commits and force-push. look at the changes
from https://github.com/plone/volto/pull/5863/files and tell me if this is the latest status

[victorchrollo14]

@ichim-david. I just tried it locally earlier, I've pushed it now.

[ichim-david]

@ichim-david. I just tried it locally earlier, I've pushed it now.

React select has the default max-height of 300px set. You would set it to 13em which I think would be calculated from
14px as the base font size so 182px. It might be too low of a number and I'm thinking of there aren't ways to call the widget for different components with a number that makes sense for it. You also have the option of using CSS selectors for specific sections and modifying the max-height.

I'm not comfortable with thinking this is a good solution to basically say that anytime you use the select widget it has a max height of 182px instead of the default 300. Maybe 200, 250px might be a better default for us but again I am always in favor of not messing a lot with the defaults and modifying those defaults as needed per context.

If we do set the maxMenuHeight and we decide on a default that we think is more sensible than 300 we still need to ensure that someone can pass a prop and modify this value.

[stevepiercy]

Initially I tried adding a maxMenuHeight prop to select component which fixes the issue in /controlpanel/dexterity-types/, but it doesn't work in all the places for instances you can check out /controlpanel/user where the issue persists.

For this scenario, instead of the select menu popping downward, and assuming it uses popper.js, it should automatically detect in which direction it should pop up. Does this select menu not use popper.js? Here are docs for reference.

https://floating-ui.com/docs/react

See also related issue and pull requests:

• UI Issue : dropdown popup positioning in contents page #5566
• fix: dropdown popup positioning in contents page #5571 (abandoned, did not pass tests)
• Fixed #5566 dropdown popup positioning in contents page #5622 (awaiting review by maintainer)

I agree with @ichim-david that changing defaults would be a last resort because of its impacts throughout the UI.

Finally, what happened to using menuPortalTarget={document.body}?

[ichim-david]

Initially I tried adding a maxMenuHeight prop to select component which fixes the issue in /controlpanel/dexterity-types/, but it doesn't work in all the places for instances you can check out /controlpanel/user where the issue persists.

For this scenario, instead of the select menu popping downward, and assuming it uses popper.js, it should automatically detect in which direction it should pop up. Does this select menu not use popper.js? Here are docs for reference.

https://floating-ui.com/docs/react

See also related issue and pull requests:

* [UI Issue : dropdown popup positioning in contents page #5566](https://github.com/plone/volto/issues/5566)

* [fix: dropdown popup positioning in contents page #5571](https://github.com/plone/volto/pull/5571) (abandoned, did not pass tests)

* [Fixed #5566 dropdown popup positioning in contents page #5622](https://github.com/plone/volto/pull/5622) (awaiting review by maintainer)

I agree with @ichim-david that changing defaults would be a last resort because of its impacts throughout the UI.

Finally, what happened to using menuPortalTarget={document.body}?

@stevepiercy it's using react-select library.
Using the target with document.body is not a good idea as some of these selects show up inside a modal.
The modals should trap the focus and ensure that you cannot focus to things outside of the modal.
If you would mount to the document.body it would add it to the end of the body and as such would
go against focusing only items inside the modal.

[stevepiercy]

@ichim-david would this example work? https://react-select.com/advanced#portaling. I asked in #4058 (comment), but that seems to have been lost in this PR. @victorchrollo14?

[victorchrollo14]

@ichim-david. I just tried it locally earlier, I've pushed it now.

React select has the default max-height of 300px set. You would set it to 13em which I think would be calculated from 14px as the base font size so 182px. It might be too low of a number and I'm thinking of there aren't ways to call the widget for different components with a number that makes sense for it. You also have the option of using CSS selectors for specific sections and modifying the max-height.

I'm not comfortable with thinking this is a good solution to basically say that anytime you use the select widget it has a max height of 182px instead of the default 300. Maybe 200, 250px might be a better default for us but again I am always in favor of not messing a lot with the defaults and modifying those defaults as needed per context.

If we do set the maxMenuHeight and we decide on a default that we think is more sensible than 300 we still need to ensure that someone can pass a prop and modify this value.

@ichim-david yeah, I agree. putting a default value would affect different parts of the UI. if we are not aware of all the places it is being used, we might break some ui as well.
react-select allows us to add custom css styling by passing in a styles prop. which we can edit in Widgets/SelectStyling.jsx. I added a max-height in there, it doesn't work as expected. The only way to add maxHeight properly is to pass the maxMenuHeight Prop.

adding maxHeight in css makes it look like below.

In the route http://localhost:3000/controlpanel/date-and-time. Although the default max-height of menu in react-select is 300.
here it exceeds that number to 501px.
The default height of 300 works until the size of the options prop is 25, but when the options size is greater than 25 then the size of menu goes to 501px. What should we do about this issue?

In order to fix the issue with schema modal. We could go with a default of 215px, which is the maximum maxMenuHeight we can put for the modal in http://localhost:3000/controlpanel/dexterity-types/*/schema such that the menu stays within the modal. Along with that we can make sure to add a prop inside SelectWidget to modify the maxMenuHeight.

[victorchrollo14]

@ichim-david would this example work? https://react-select.com/advanced#portaling. I asked in #4058 (comment), but that seems to have been lost in this PR.

@victorchrollo14?
I used it in the initial commits for this PR, then removed it as it wasn't really that good of a solution.