All ASCellNode nodes to be non accessible if needed #1941
Merged
Conversation
This file contains 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
…roperties from its Element's node
|
|
bolsinga
approved these changes
Dec 16, 2020
sreshetnyak
added a commit
to sreshetnyak/Texture
that referenced
this pull request
Dec 24, 2021
…extureGroup#1888) We did not notice any effect on performance of the Pinterest app by not caching `accessibilityElements` in `_ASDisplayView`. By not caching the elements, we can be sure that the elements will be correct even when nodes change visibility state. There will be a performance impact when voice over is enabled, but providing the correct elements for the current state of a view is more important than performance in this case. TextureGroup#1853 Remove background deallocation helper code (TextureGroup#1890) * Remove background deallocation helper code Last use removed in Texture with TextureGroup#1840, now PINS no longer uses it either. Less OOMs is so nice. * remove methods from docs Call will / did display node for ASTextNode. Fixes TextureGroup#1680 (TextureGroup#1893) Do not expose tgmath.h to all clients of Texture (TextureGroup#1900) * Do not expose tgmath.h to all clients of Texture - tgmath.h #undef the `log` macro for mathematical reasons. Code that may also use a log name (such as CocoaLumberjack) will get confused by this when they try to use `NS_SWIFT_NAME` with `log` as part of the name. - `ABS` from NSObjCRuntime.h is what is typically used for abs on `CGFloat`. - Note: removing tgmath.h from the Texture umbrella header may expose clients that implicitly depended upon it being imported. Sources may have to be updated after this to `#import <tgmath.h>` explicitly. Disable text kit lock (TextureGroup#1910) * Add experiment to disable global textkit lock * Forgot the bang Add an experiment that makes ASDataController to do everything on main thread (TextureGroup#1911) * Add an experiment that makes ASDataController to do everything on main thread Under this experiment, ASDataController will allocate and layout all nodes on the main thread. This helps to avoid deadlocks that would otherwise occur if some of the node allocations or layouts caused ASDataController's background queue to block on main thread. As a bonus, this experiment also helps to measure how much performance wins we get from doing the work off main. * Remove ASSERT_ON_EDITING_QUEUE More on ASDataController's main-thread-only mode (TextureGroup#1915) Follow up on TextureGroup#1911: it's not enough to execute step 3 on the main thread because -_allocateNodesFromElements: uses ASDispatchApply to offload the work to other threads. So this diff adds a flag to tell that method to do everything serially on the calling thread. Fix failing ASConfigurationTests (TextureGroup#1923) * Fix failing ASConfigurationTests * Update configuration.json as well Fix hit point when ASCollectionNode inverted set to true (TextureGroup#1781) * Account for possible inverted transform during hit test When ASCollectionNode has the `inverted` flag set, a transform gets set on the cell node. We need to make sure that we account for that when dealing in the view coordinates. * Store self.node and self.node.view in local variables for better readability. * Add a test for hit testing in an inverted ASCollectionNode Ship ASExperimentalDispatchApply (TextureGroup#1924) Closes TextureGroup#1850. [ASTextNode2] Make some ASTextNode2 layout files public (TextureGroup#1939) * Trying to make ASTextLinePositionModifier public * d’oh * be a little more restrictive on the files we pull into the pod * Never mind, I guess we need all of these * update the project file as well * try this again * I think this will work this time. Set ASTableView isAccessibilityElement, accessibilityElementsHidden properties from its Element's node (TextureGroup#1941) Update CocoaPods to use the CDN instead of the old trunk repo. (TextureGroup#1957) Remove redundant assignment. (TextureGroup#1960) Fix mutation of variable that is never read. (TextureGroup#1961) use https for slack link (TextureGroup#1950) fix missing hidden class (TextureGroup#1952) Fix WKWebView Accessibility (TextureGroup#1955) * Return nil instead of empty array when no accessibility elements are found. Fixes TextureGroup#1954. * Use nullability annotations to fix static analyzer warnings. * Add UI test target. * Add UI test to make sure web view stays accessible. * Revert "Add UI test to make sure web view stays accessible." This reverts commit 00253f4. * Revert "Add UI test target." This reverts commit 288b5e0. * Add unit test to make sure accessibility elements are correct when a WKWebView is wrapped in an ASDisplayNode. Update asdkGram swift sample to swift version 5.3 (TextureGroup#1962) Fix order-dependent ASTextNodeTests (TextureGroup#1963) Exposes a new option in ASImageDownloaderProtocol to retry image downloads (TextureGroup#1948) * Exposes a new option in ASImageDownloaderProtocol to retry image downloads At the moment the ASBasicImageDownloader does not automatically retry image downloads if the remote host is unreachable. On the contrary the ASPINRemoteImageDownloader automatically retries. Retrying is something that ultimately clients need to be able to control, for example to fail fast to an alternative image rather than keep retrying for more than one minute while not displaying any image. This change exposes a new option in the ASImageDownloaderProtocol to retry image downloads. It also uses this new option in both ASNetworkImageNode and also ASMultiplexImageNode, setting it to YES to preserve the current behaviour. * Fixes a failing test in ASMultiplexImageNodeTests * Fixes ScreenNode.m ScreenNode.m is implementing ASImageDownloaderProtocol and needs to be fixed to reflect changes in the latter. Add experiment to ensure ASCollectionView's range controller updates on changeset updates (TextureGroup#1976) This experiment makes sure a ASCollectionView's `rangeController` updates when a changeset WITH updates is applied. Currently it is possible for nodes inserted into the preload range to not get preloaded when performing a batch update. For example, suppose a collection node has: - Tuning parameters with a preload range of 1 screenful for the given range mode. - Nodes A and B where A is visible and B is off screen. Currently if node B is deleted and a new node C is inserted in its place, node C will not get preloaded until the collection node is scrolled. This is because the preloading mechanism relies on a `setNeedsUpdate` call on the range controller as part of the `-collectionView:willDisplayCell:forItemAtIndexPath:` delegate method when the batch update is submitted. However, in the example outlined above, this sometimes doesn't happen automtically, causing the range update to be delayed until the next the view scrolls. Remove Facebook and shift everything around, add Remix by Buffer (TextureGroup#1978) Expand ASExperimentalRangeUpdateOnChangesetUpdate to ASTableView (TextureGroup#1979) A previous commit (TextureGroup@8f7444e) aimed to fix a preloading bug for ASCollectionView. This commit expands this fix to ASTableView as the bug occurs there too. Previous commit message for context: This experiment makes sure a ASCollectionView's `rangeController` updates when a changeset WITH updates is applied. Currently it is possible for nodes inserted into the preload range to not get preloaded when performing a batch update. For example, suppose a collection node has: - Tuning parameters with a preload range of 1 screenful for the given range mode. - Nodes A and B where A is visible and B is off screen. Currently if node B is deleted and a new node C is inserted in its place, node C will not get preloaded until the collection node is scrolled. This is because the preloading mechanism relies on a `setNeedsUpdate` call on the range controller as part of the `-collectionView:willDisplayCell:forItemAtIndexPath:` delegate method when the batch update is submitted. However, in the example outlined above, this sometimes doesn't happen automtically, causing the range update to be delayed until the next the view scrolls. Remove trailing semicolons between method parameters and body (TextureGroup#1973) Having a semi-colon between a method parameters list and a method body is not not correct and is usually caused by a copy and paste error while creating the method definition from its declaration. Fixes the following compilation warnings when building with -Wsemicolon-before-method-body (which is part of -Wextra): ASPINRemoteImageDownloader.mm:230:85: error: semicolon before method body is ignored [-Werror,-Wsemicolon-before-method-body] - (id <ASImageContainerProtocol>)synchronouslyFetchedCachedImageWithURL:(NSURL *)URL; ^ ASPINRemoteImageDownloader.mm:275:76: error: semicolon before method body is ignored [-Werror,-Wsemicolon-before-method-body] completion:(ASImageDownloaderCompletion)completion; ^ 2 errors generated. Fixes applied to both code, examples and samples in documentation. [Layout] Add RTL support to LayoutSpecs (TextureGroup#1983) * [Layout] Add RTL support to LayoutSpecs This is largely a slight update for TextureGroup#1805. If RTL is enabled, `calculateLayoutLayoutSpec:` will flip the origin of all sublayouts. The new part of the diff is that ASBatchFetching now supports proper fetching on RTL horizontal scrollViews. * Fix build and add RTL batch fetching tests [RTL/Batching] Make ASDisplayShouldFetchBatchForScrollView aware of flipped CV layouts (TextureGroup#1985) * [RTL/Batching] Make ASDisplayShouldFetchBatchForScrollView aware of flipped CV layouts UICollectionViewLayout has a property called `flipsHorizontallyInOppositeLayoutDirection`. If this is set to `YES` then a RTL collectionView’s contentOffset behaves like it does in LTR. In other words, the first item is at contentOffset 0. In this case, the existing logic for `ASDisplayShouldFetchBatchForScrollView` works in RTL. If you don’t override `flipsHorizontallyInOppositeLayoutDirection` to be `YES`, then it means that in RTL languages the first item in your collectionView will actually be at x offset `collectionView.contentSize.width - collectionView.frame.size.width`. As you scroll to the right, the content offset will decrease until you reach the end of the data at a content offset of 0,0. In this case, `ASDisplayShouldFetchBatchForScrollView` needs to know that you are in RTL and the layout is not flipped. It can then use the contentOffset as the `remainingDistance` to determine when to fetch. * fix indentation * assert that we are on main when accessing CV layout [RTL] Guard access of flipsHorizontallyInOppositeLayoutDirection for iOS >= 11 (TextureGroup#2003) `flipsHorizontallyInOppositeLayoutDirection` is available in iOS11 and greater. Texture still supports iOS9 so we need to make sure not to call this it in those cases. Rename ASNavigationController to ASDKNavigationController to fix name collision (TextureGroup#2020) As of iOS15 the AuthenticationServices framework has a class named `ASNavigationController`. We need to rename our `ASNavigationController` to protect against undefined behavior. Note: This change was based on this PR TextureGroup#2014. We were slow in merging it and the author has not replied, so I'm making a new one to get this landed. [3.1.0] Create new version of ASDK (TextureGroup#2021) With the breaking change of renaming ASNavigationController to ASDKNavigationController, we have released a new version of Texture. Please see `ThreeMigrationGuide.md` for how to handle the breaking changes in 3.1.0. [3.1.0] Update CHANGELOG [3.1.0] Update .github_changelog_generator Update RELEASE.md Remove AssetsLibrary dependency for tvOS (TextureGroup#2034) - The framework isn't available on tvOS. This causes CocoaPods linting to fail which prevented me from pushing the new release out. - One way to fix this is to have a different `default_subspecs` for tvOS that doesn't have AssetsLibrary subspec, but per-platform `default_subspecs` doesn't seem to be supported by CocoaPods. So I updated the subspec itself to only depend on the framework for iOS. This means the subspec is empty/useless for tvOS (and other platforms FWIW). - Tested with `pod spec lint Texture.podspec`. - Fixes TextureGroup#1992 and part of TextureGroup#1549. Also unblocks 3.1.0 release. - For the long term, we can remove the subspec entirely when iOS 9 is deprecated (TextureGroup#1828).
sreshetnyak
added a commit
to sreshetnyak/Texture
that referenced
this pull request
Dec 27, 2021
…extureGroup#1888) We did not notice any effect on performance of the Pinterest app by not caching `accessibilityElements` in `_ASDisplayView`. By not caching the elements, we can be sure that the elements will be correct even when nodes change visibility state. There will be a performance impact when voice over is enabled, but providing the correct elements for the current state of a view is more important than performance in this case. TextureGroup#1853 Remove background deallocation helper code (TextureGroup#1890) * Remove background deallocation helper code Last use removed in Texture with TextureGroup#1840, now PINS no longer uses it either. Less OOMs is so nice. * remove methods from docs Call will / did display node for ASTextNode. Fixes TextureGroup#1680 (TextureGroup#1893) Do not expose tgmath.h to all clients of Texture (TextureGroup#1900) * Do not expose tgmath.h to all clients of Texture - tgmath.h #undef the `log` macro for mathematical reasons. Code that may also use a log name (such as CocoaLumberjack) will get confused by this when they try to use `NS_SWIFT_NAME` with `log` as part of the name. - `ABS` from NSObjCRuntime.h is what is typically used for abs on `CGFloat`. - Note: removing tgmath.h from the Texture umbrella header may expose clients that implicitly depended upon it being imported. Sources may have to be updated after this to `#import <tgmath.h>` explicitly. Disable text kit lock (TextureGroup#1910) * Add experiment to disable global textkit lock * Forgot the bang Add an experiment that makes ASDataController to do everything on main thread (TextureGroup#1911) * Add an experiment that makes ASDataController to do everything on main thread Under this experiment, ASDataController will allocate and layout all nodes on the main thread. This helps to avoid deadlocks that would otherwise occur if some of the node allocations or layouts caused ASDataController's background queue to block on main thread. As a bonus, this experiment also helps to measure how much performance wins we get from doing the work off main. * Remove ASSERT_ON_EDITING_QUEUE More on ASDataController's main-thread-only mode (TextureGroup#1915) Follow up on TextureGroup#1911: it's not enough to execute step 3 on the main thread because -_allocateNodesFromElements: uses ASDispatchApply to offload the work to other threads. So this diff adds a flag to tell that method to do everything serially on the calling thread. Fix failing ASConfigurationTests (TextureGroup#1923) * Fix failing ASConfigurationTests * Update configuration.json as well Fix hit point when ASCollectionNode inverted set to true (TextureGroup#1781) * Account for possible inverted transform during hit test When ASCollectionNode has the `inverted` flag set, a transform gets set on the cell node. We need to make sure that we account for that when dealing in the view coordinates. * Store self.node and self.node.view in local variables for better readability. * Add a test for hit testing in an inverted ASCollectionNode Ship ASExperimentalDispatchApply (TextureGroup#1924) Closes TextureGroup#1850. [ASTextNode2] Make some ASTextNode2 layout files public (TextureGroup#1939) * Trying to make ASTextLinePositionModifier public * d’oh * be a little more restrictive on the files we pull into the pod * Never mind, I guess we need all of these * update the project file as well * try this again * I think this will work this time. Set ASTableView isAccessibilityElement, accessibilityElementsHidden properties from its Element's node (TextureGroup#1941) Update CocoaPods to use the CDN instead of the old trunk repo. (TextureGroup#1957) Remove redundant assignment. (TextureGroup#1960) Fix mutation of variable that is never read. (TextureGroup#1961) use https for slack link (TextureGroup#1950) fix missing hidden class (TextureGroup#1952) Fix WKWebView Accessibility (TextureGroup#1955) * Return nil instead of empty array when no accessibility elements are found. Fixes TextureGroup#1954. * Use nullability annotations to fix static analyzer warnings. * Add UI test target. * Add UI test to make sure web view stays accessible. * Revert "Add UI test to make sure web view stays accessible." This reverts commit 00253f4. * Revert "Add UI test target." This reverts commit 288b5e0. * Add unit test to make sure accessibility elements are correct when a WKWebView is wrapped in an ASDisplayNode. Update asdkGram swift sample to swift version 5.3 (TextureGroup#1962) Fix order-dependent ASTextNodeTests (TextureGroup#1963) Exposes a new option in ASImageDownloaderProtocol to retry image downloads (TextureGroup#1948) * Exposes a new option in ASImageDownloaderProtocol to retry image downloads At the moment the ASBasicImageDownloader does not automatically retry image downloads if the remote host is unreachable. On the contrary the ASPINRemoteImageDownloader automatically retries. Retrying is something that ultimately clients need to be able to control, for example to fail fast to an alternative image rather than keep retrying for more than one minute while not displaying any image. This change exposes a new option in the ASImageDownloaderProtocol to retry image downloads. It also uses this new option in both ASNetworkImageNode and also ASMultiplexImageNode, setting it to YES to preserve the current behaviour. * Fixes a failing test in ASMultiplexImageNodeTests * Fixes ScreenNode.m ScreenNode.m is implementing ASImageDownloaderProtocol and needs to be fixed to reflect changes in the latter. Add experiment to ensure ASCollectionView's range controller updates on changeset updates (TextureGroup#1976) This experiment makes sure a ASCollectionView's `rangeController` updates when a changeset WITH updates is applied. Currently it is possible for nodes inserted into the preload range to not get preloaded when performing a batch update. For example, suppose a collection node has: - Tuning parameters with a preload range of 1 screenful for the given range mode. - Nodes A and B where A is visible and B is off screen. Currently if node B is deleted and a new node C is inserted in its place, node C will not get preloaded until the collection node is scrolled. This is because the preloading mechanism relies on a `setNeedsUpdate` call on the range controller as part of the `-collectionView:willDisplayCell:forItemAtIndexPath:` delegate method when the batch update is submitted. However, in the example outlined above, this sometimes doesn't happen automtically, causing the range update to be delayed until the next the view scrolls. Remove Facebook and shift everything around, add Remix by Buffer (TextureGroup#1978) Expand ASExperimentalRangeUpdateOnChangesetUpdate to ASTableView (TextureGroup#1979) A previous commit (TextureGroup@8f7444e) aimed to fix a preloading bug for ASCollectionView. This commit expands this fix to ASTableView as the bug occurs there too. Previous commit message for context: This experiment makes sure a ASCollectionView's `rangeController` updates when a changeset WITH updates is applied. Currently it is possible for nodes inserted into the preload range to not get preloaded when performing a batch update. For example, suppose a collection node has: - Tuning parameters with a preload range of 1 screenful for the given range mode. - Nodes A and B where A is visible and B is off screen. Currently if node B is deleted and a new node C is inserted in its place, node C will not get preloaded until the collection node is scrolled. This is because the preloading mechanism relies on a `setNeedsUpdate` call on the range controller as part of the `-collectionView:willDisplayCell:forItemAtIndexPath:` delegate method when the batch update is submitted. However, in the example outlined above, this sometimes doesn't happen automtically, causing the range update to be delayed until the next the view scrolls. Remove trailing semicolons between method parameters and body (TextureGroup#1973) Having a semi-colon between a method parameters list and a method body is not not correct and is usually caused by a copy and paste error while creating the method definition from its declaration. Fixes the following compilation warnings when building with -Wsemicolon-before-method-body (which is part of -Wextra): ASPINRemoteImageDownloader.mm:230:85: error: semicolon before method body is ignored [-Werror,-Wsemicolon-before-method-body] - (id <ASImageContainerProtocol>)synchronouslyFetchedCachedImageWithURL:(NSURL *)URL; ^ ASPINRemoteImageDownloader.mm:275:76: error: semicolon before method body is ignored [-Werror,-Wsemicolon-before-method-body] completion:(ASImageDownloaderCompletion)completion; ^ 2 errors generated. Fixes applied to both code, examples and samples in documentation. [Layout] Add RTL support to LayoutSpecs (TextureGroup#1983) * [Layout] Add RTL support to LayoutSpecs This is largely a slight update for TextureGroup#1805. If RTL is enabled, `calculateLayoutLayoutSpec:` will flip the origin of all sublayouts. The new part of the diff is that ASBatchFetching now supports proper fetching on RTL horizontal scrollViews. * Fix build and add RTL batch fetching tests [RTL/Batching] Make ASDisplayShouldFetchBatchForScrollView aware of flipped CV layouts (TextureGroup#1985) * [RTL/Batching] Make ASDisplayShouldFetchBatchForScrollView aware of flipped CV layouts UICollectionViewLayout has a property called `flipsHorizontallyInOppositeLayoutDirection`. If this is set to `YES` then a RTL collectionView’s contentOffset behaves like it does in LTR. In other words, the first item is at contentOffset 0. In this case, the existing logic for `ASDisplayShouldFetchBatchForScrollView` works in RTL. If you don’t override `flipsHorizontallyInOppositeLayoutDirection` to be `YES`, then it means that in RTL languages the first item in your collectionView will actually be at x offset `collectionView.contentSize.width - collectionView.frame.size.width`. As you scroll to the right, the content offset will decrease until you reach the end of the data at a content offset of 0,0. In this case, `ASDisplayShouldFetchBatchForScrollView` needs to know that you are in RTL and the layout is not flipped. It can then use the contentOffset as the `remainingDistance` to determine when to fetch. * fix indentation * assert that we are on main when accessing CV layout [RTL] Guard access of flipsHorizontallyInOppositeLayoutDirection for iOS >= 11 (TextureGroup#2003) `flipsHorizontallyInOppositeLayoutDirection` is available in iOS11 and greater. Texture still supports iOS9 so we need to make sure not to call this it in those cases. Rename ASNavigationController to ASDKNavigationController to fix name collision (TextureGroup#2020) As of iOS15 the AuthenticationServices framework has a class named `ASNavigationController`. We need to rename our `ASNavigationController` to protect against undefined behavior. Note: This change was based on this PR TextureGroup#2014. We were slow in merging it and the author has not replied, so I'm making a new one to get this landed. [3.1.0] Create new version of ASDK (TextureGroup#2021) With the breaking change of renaming ASNavigationController to ASDKNavigationController, we have released a new version of Texture. Please see `ThreeMigrationGuide.md` for how to handle the breaking changes in 3.1.0. [3.1.0] Update CHANGELOG [3.1.0] Update .github_changelog_generator Update RELEASE.md Remove AssetsLibrary dependency for tvOS (TextureGroup#2034) - The framework isn't available on tvOS. This causes CocoaPods linting to fail which prevented me from pushing the new release out. - One way to fix this is to have a different `default_subspecs` for tvOS that doesn't have AssetsLibrary subspec, but per-platform `default_subspecs` doesn't seem to be supported by CocoaPods. So I updated the subspec itself to only depend on the framework for iOS. This means the subspec is empty/useless for tvOS (and other platforms FWIW). - Tested with `pod spec lint Texture.podspec`. - Fixes TextureGroup#1992 and part of TextureGroup#1549. Also unblocks 3.1.0 release. - For the long term, we can remove the subspec entirely when iOS 9 is deprecated (TextureGroup#1828).
|
@decim92 This change has caused some issues for us and others (see #1997). If I set my What I'm wondering is more about why this change was necessary for you, since |
bdolman
added a commit
to Hitlabs/Texture
that referenced
this pull request
Apr 18, 2022
This is a partial revert of TextureGroup#1941 The change in TextureGroup#1941 has caused some issues for us and others (see TextureGroup#1997). If I set `myNode.isAccessibilityElement = true,` then what ends up happening is that the table cell wrapper view (_ASTableViewCell) also has `isAccessibilityElement = true`. But then when iOS goes through the hierarchy it identifies the table cell wrapper view as the target and attempts to use that accessibilityLabel, which doesn't exist (because it's actually on the nested node view). So our node's label is never used. I'm not sure why setting `isAccessibilityElement` in `_ASTableViewCell` was necessary in TextureGroup#1941 in order to make a node non-accessible, since the default value is already false.
rcasula
pushed a commit
to openbitapp/Texture
that referenced
this pull request
Dec 12, 2023
This is a partial revert of TextureGroup#1941 The change in TextureGroup#1941 has caused some issues for us and others (see TextureGroup#1997). If I set `myNode.isAccessibilityElement = true,` then what ends up happening is that the table cell wrapper view (_ASTableViewCell) also has `isAccessibilityElement = true`. But then when iOS goes through the hierarchy it identifies the table cell wrapper view as the target and attempts to use that accessibilityLabel, which doesn't exist (because it's actually on the nested node view). So our node's label is never used. I'm not sure why setting `isAccessibilityElement` in `_ASTableViewCell` was necessary in TextureGroup#1941 in order to make a node non-accessible, since the default value is already false.
paulgee31
reviewed
Nov 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently there's no way to make a ASCellNode node non accessible, this change solves the problem for cases when you only want the cell node to be a decorative view like a custom separator between cells