Skip to content

remove DefaultViewPlugins and move implementation into ViewController#689

Merged
tmarmer merged 6 commits intomainfrom
move-view-plugins
Jul 16, 2025
Merged

remove DefaultViewPlugins and move implementation into ViewController#689
tmarmer merged 6 commits intomainfrom
move-view-plugins

Conversation

@tmarmer
Copy link
Copy Markdown
Contributor

@tmarmer tmarmer commented Jul 15, 2025

  • Remove DefaultViewPlugins
  • Add all view plugins into the ViewController and ensure they get added before the this.hooks.view is called
    • This should ensure a consistent tap order for built in view plugins

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
  • N/A

Does your PR have any documentation updates?

  • Updated docs
  • No Update needed
  • Unable to update docs

Release Notes

  • Remove DefaultViewPlugins and add all plugins it uses into the ViewController
  • Add and export types for the hooks on the ViewController and ViewInstance
📦 Published PR as canary version: 0.12.1--canary.689.24630

Try this version out locally by upgrading relevant packages to 0.12.1--canary.689.24630

@tmarmer tmarmer requested a review from a team as a code owner July 15, 2025 20:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.05%. Comparing base (d686342) to head (6af507e).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #689   +/-   ##
=======================================
  Coverage   90.05%   90.05%           
=======================================
  Files         334      333    -1     
  Lines       21017    21012    -5     
  Branches     2104     2101    -3     
=======================================
- Hits        18926    18923    -3     
+ Misses       2073     2071    -2     
  Partials       18       18           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread core/player/src/controllers/view/controller.ts Outdated
}
}

private createViewPlugins(): Array<ViewPlugin> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can this function just apply the plugins?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apply needs to happen on during the onView instance whenever a new ViewInstance is created. If we want to only construct the plugins once and apply them to each view then we need to just create them first. If we're okay with constructing and applying them each time then I can make just one function to do both.

Comment thread core/player/src/view/view.ts Outdated
@tmarmer tmarmer force-pushed the move-view-plugins branch from b4bb79f to 6af507e Compare July 16, 2025 17:02
@tmarmer
Copy link
Copy Markdown
Contributor Author

tmarmer commented Jul 16, 2025

/canary

@KetanReddy KetanReddy added the patch Increment the patch version when merged label Jul 16, 2025
@tmarmer tmarmer merged commit c013275 into main Jul 16, 2025
11 checks passed
@tmarmer tmarmer deleted the move-view-plugins branch July 16, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch Increment the patch version when merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants