Skip to content

chore(feed): delete the dead _LoadingIndicator - #6559

Merged
hm21 merged 1 commit into
mainfrom
chore/6492-delete-dead-loading-indicator
Jul 31, 2026
Merged

chore(feed): delete the dead _LoadingIndicator#6559
hm21 merged 1 commit into
mainfrom
chore/6492-delete-dead-loading-indicator

Conversation

@hm21

@hm21 hm21 commented Jul 31, 2026

Copy link
Copy Markdown
Member

Description

Deletes the private _LoadingIndicator / _LoadingIndicatorState pair in mobile/lib/screens/feed/pooled_fullscreen_video_feed_screen.dart. It is never instantiated — the only four references in the repo were its own class declaration, constructor, createState, and the State class. It is file-private and the file has no part directive, so nothing outside could reach it either.

The other _LoadingIndicators that grep turns up (add_to_list_dialog.dart, camera_permission_gate.dart, video_loading_placeholder.dart) are separate file-scoped classes and all in use.

Why delete rather than polish: the widget carried the last non-doc reference to the seek-based loop enforcement removed in #6489 — its _delay constant was justified with "flashes that occur during play/pause and loop-enforcement seeks". Trimming that comment was suggested during review of #6489, but touching up a widget that never builds is the wrong fix.

Related Issue: Closes #6492

Out of Scope

None. Pure deletion, no behaviour change.

Verification

  • Confirmed no imports go stale: dart:async is still needed for unawaited/Future, and BrandedLoadingIndicator is still used at two other call sites in the same file.
  • flutter analyze on the changed file — no issues.
  • flutter test test/screens/feed/pooled_fullscreen_video_feed_screen_test.dart test/screens/feed/pooled_fullscreen_video_feed_args_test.dart test/screens/feed/video_tap_shield_test.dart test/screens/feed/keyboard_aware_top_fade_test.dart — 47/47 pass.
  • No device verification: the deleted widget never built, so there is nothing observable to check on a real device.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

The private _LoadingIndicator/_LoadingIndicatorState pair in
pooled_fullscreen_video_feed_screen.dart was never instantiated. It is
file-private and the file has no part directive, so nothing outside can
reach it either.

It also carried the last non-doc reference to the seek-based loop
enforcement removed in #6489: its _delay constant was justified with
"flashes that occur during play/pause and loop-enforcement seeks".
Deleting the widget is the right fix rather than trimming a comment on
code that never builds.
@hm21 hm21 self-assigned this Jul 31, 2026
@hm21
hm21 requested review from mbradley and realmeylisdev July 31, 2026 16:19
@hm21
hm21 marked this pull request as ready for review July 31, 2026 16:19
@github-actions

Copy link
Copy Markdown

Mobile PR Preview

Preview refreshed for 0926183

Last refresh: 0926183 at 2026-07-31 16:24:21 UTC (preview run)

Property Value
Preview URL https://adee7b4f.openvine-app.pages.dev
Pages project openvine-app
Preview branch pr-6559
PR branch chore/6492-delete-dead-loading-indicator
Commit 0926183

@mbradley mbradley left a comment

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.

Verified the deletion independently: the four _LoadingIndicator references on main are all self-referential (declaration, constructor, createState, State class), the file has no part directive so nothing external could reach it, and the three same-named classes elsewhere (add_to_list_dialog.dart, camera_permission_gate.dart, video_loading_placeholder.dart) are separate file-scoped widgets that are all instantiated. dart:async stays load-bearing via unawaited, and BrandedLoadingIndicator is still used at lines 711 and 1033. Analyze clean and 47/47 on the four cited test files locally.

Also traced why it was dead, since a deletion is the wrong move if a call site was dropped by accident. It wasn't: d1486f3e8 (#4682, native-video migration) removed the entire containing widget wholesale and left this pair orphaned. The affordance itself moved rather than disappearing — this screen renders through FeedVideos, which renders VideoLoadingPlaceholder. So there's no regression being frozen in place here.

@hm21
hm21 merged commit f8373fa into main Jul 31, 2026
16 of 17 checks passed
@hm21
hm21 deleted the chore/6492-delete-dead-loading-indicator branch July 31, 2026 16:34
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.

chore(feed): delete the dead _LoadingIndicator in pooled_fullscreen_video_feed_screen

2 participants