ScrollView - Respect the shows<Horizontal | Vertical>ScrollIndicator props on macOS
#1135
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 tasks
|
why isn't ios and macos property aligned? macOS can't just use "showsVerticalScrollIndicator" instead? |
harrieshin
approved these changes
Apr 25, 2022
They are aligned? Setting "showsVerticalScrollIndicator" will internally set "hasVerticalScroller" on NSScrollView. Before we would overwrite that value everytime you lay out the scroll view, which we don't do anymore after this PR. |
ah i misunderstood. hasVerticalScroller is actually a NSScrollView property not our own custom property |
shwanton
added a commit
to shwanton/react-native-macos
that referenced
this pull request
Feb 13, 2023
Summary: ## Sync OS changes for ScrollBar [[ScrollView] Don't always adjust content view padding for both scrollers microsoft#1132](microsoft#1132) [ScrollView - Respect the shows<Horizontal | Vertical>ScrollIndicator props on macOS microsoft#1135](microsoft#1135) [Remove horizontal scrollview indicator padding when empty microsoft#1635](microsoft#1635) [Add ScrollView Indicator Overlay style microsoft#1658](microsoft#1658) NOTE: This will override the "Always" user setting and force the scrollview to use the overlay style instead {F850963270} Test Plan: |ScrollView Overlay Style| |https://pxl.cl/2qp0D| |ScrollView works correctly| |https://pxl.cl/2qpd1| Reviewers: chpurrer, lefever Reviewed By: chpurrer Differential Revision: https://phabricator.intern.facebook.com/D42637723
shwanton
added a commit
to shwanton/react-native-macos
that referenced
this pull request
Mar 10, 2023
Summary: ## Sync OS changes for ScrollBar [[ScrollView] Don't always adjust content view padding for both scrollers microsoft#1132](microsoft#1132) [ScrollView - Respect the shows<Horizontal | Vertical>ScrollIndicator props on macOS microsoft#1135](microsoft#1135) [Remove horizontal scrollview indicator padding when empty microsoft#1635](microsoft#1635) [Add ScrollView Indicator Overlay style microsoft#1658](microsoft#1658) NOTE: This will override the "Always" user setting and force the scrollview to use the overlay style instead {F850963270} Test Plan: |ScrollView Overlay Style| |https://pxl.cl/2qp0D| |ScrollView works correctly| |https://pxl.cl/2qpd1| Reviewers: chpurrer, lefever Reviewed By: chpurrer Subscribers: generatedunixname89002005327315 Differential Revision: https://phabricator.intern.facebook.com/D42637723
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please select one of the following
Summary
React Native's ScrollView component defines two props for iOS and Android:
showsVerticalScrollIndicatorandshowsHorizontalScrollIndicator. These are designed to be user overridable (I.E: the user could make a vertical scrolling list that never shows a vertical indicator). macOS has equivalent props:hasVerticalScrollerandhasVerticalScroller, but we would set them ourselves in code and not respect the user choice. We can simplify our code and respect the user choice by removing bits where we manually set whether our scroll view has vertical and horizontal scrollers or not. I also setautoHidesScrollbars = trueto better match the existing behavior and because I think it's a better default to have.Changelog
[macOS] [Fixed] - ScrollView respects props showsVerticalScrollIndicator, showsHorizontalScrollIndicator
Test Plan
I tested various different configurations of Scrollbars, making use of the existing Scrollindicators test". One thing to note is that macOS has 2 types of scrollbars that you can switch between in user preferences:
I made sure that the proper margins are set so that the content view is sized appropriately based on which scrollbars are current. I also tested fixed size vs variable size scrollviews by locally hardcoding a size in RNTester.
Screen.Recording.2022-04-25.at.4.13.10.AM.mov