Enhancement: Makes usePreventScroll controllable.

[McZenith]

Closes #889

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

[LFDanLu]

Thanks for working on this! Just a couple of suggestions that should fix the tests, lemme know if they make sense

[LFDanLu]

Suggested change

	export function usePreventScroll(options? : PreventScrollOptions) {
	export function usePreventScroll(options? : PreventScrollOptions = {}) {

Adding the above would make it so you can properly attempt to deconstruct isOpen even when options is undefined.

[McZenith]

Thanks @LFDanLu! I was getting an error (How do I fix error TS1015: Parameter cannot have question mark and initializer?
). However, i stumbled upon this (https://stackoverflow.com/questions/17186566/how-do-i-fix-error-ts1015-parameter-cannot-have-question-mark-and-initializer) and it is all resolved now.

[LFDanLu]

Suggested change

	const {isOpen = false} = options;
	const {isOpen = true} = options;

IMO this should be flipped to true so that pre-existing components will continue to work if they call usePreventScroll without any arguments.

[devongovett]

In the issue I suggested this function usePreventScroll(options?: PreventScrollOptions = {isOpen: true}). Then, if the options aren't given, we default to true (as we do today), but if the options are given, we default to false to match expected behavior based on prop name. Too weird?

[LFDanLu]

that sounds fine but isn't that essentially what the above change is doing?

[LFDanLu]

Ah, I see the difference now (thanks @snowystinger). IMO it feels a bit weird that we would have two different defaults dependent on whether or not options are provided or not. Is there a use case you had in mind?

[devongovett]

Just backward compatibility really...

[McZenith]

Hello! I'm going to work on this tonight. I'm really sorry for the delay I've been away for days now. Thanks for your help I really appreciate.

[LFDanLu]

@McZenith No worries, thanks for working on this! You can go ahead and implement the format that @devongovett suggested above.

[devongovett]

Hey @McZenith! I've updated things a little after reconsidering the API for this to try to make it less confusing. We now have an isDisabled option rather than isOpen that is true by default (confusing). I've also added a test to ensure things are working. Hope you don't mind me pushing to your branch. Thanks again for the contribution!