Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused lightbox options #1616

Merged
merged 11 commits into from
Oct 5, 2023
Merged

Remove unused lightbox options #1616

merged 11 commits into from
Oct 5, 2023

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Oct 5, 2023

This is a purely mechanical refactor of the lightbox code. Since we vendored the code, I simplified the code structure a bit and removed the code that we don't use. This narrows down to the features we actually rely on.

  • a521860: There's no reason for these to be reusable Hooks. First, let's move them to the only file where they're used.
  • df839e9: This file is unused entirely. Delete.
  • bcfc928: We don't use the long press gesture. Delete the implementation. (We can readd later if needed.)
  • bc354bf: Double tap is always on. Hardcode the option as true.
  • 193629c: Swipe to close is always on. Hardcode the option as true.
  • 40fcf9a: We don't use this callback. Delete the option and the Effect.
  • 1234f17: Let's inline these custom Hooks directly into the component. There's no reason for them to be abstracted.
  • 0a80312: Move the nested LightboxFooter out of Lightbox since that's not valid in React. Same logic as before.
  • 5d05e6d: Inline iOS-specific useDoubleTapToZoom into the only component using it.
  • fec6fda: Remove unused utilities, move some to the only place they're used.

Tested on Android and iOS, exact same behavior like before.

@gaearon gaearon requested review from pfrazee and ansh October 5, 2023 19:52
@gaearon gaearon marked this pull request as ready for review October 5, 2023 19:52
@gaearon gaearon force-pushed the lightbox-refactor branch from 23dc955 to 5d05e6d Compare October 5, 2023 21:51
Copy link
Collaborator

@pfrazee pfrazee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on-device both platforms, no regressions found. Looks good!

@pfrazee pfrazee merged commit 260b03a into main Oct 5, 2023
@pfrazee pfrazee deleted the lightbox-refactor branch October 5, 2023 22:29
estrattonbailey added a commit that referenced this pull request Oct 9, 2023
…e-post-tags-into-app

* origin/main: (40 commits)
  1.52
  README: tweaks to high-level context (#1625)
  Fix stuck lightbox header after double tap (#1627)
  Fix: add padding to the spinner bottom while loading threads (#1626)
  Rewrite Android lightbox (#1624)
  Dont trim before posting (close #1621) (#1622)
  Only listen to back button on android (#1623)
  Improve typeahead search with inclusion of followed users (temporary solution) (#1612)
  Slightly smaller highlighted post text (#1608)
  Pull upstream bugfixes to bottom-sheet (#1606)
  Fix animations and gestures getting reset on state updates in the lightbox (#1618)
  Remove unused lightbox options (#1616)
  Profile UI tweaks (#1607)
  Fix invite codes flash on desktop, use loading placeholder (#1591)
  Update to [email protected] (#1599)
  Fixed a typo on the onboarding recommended screen (#1604)
  Onboarding & feed fixes (#1602)
  Improve time to content in the search page (#1603)
  Fix a potential reference error in bottombarweb (#1600)
  Fix: only use scroll-positioning control on thread when looking at replies (#1587)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants