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

Custom Playback Settings: Add Feature Flag and add segmented tab bar #3065

Merged
merged 6 commits into from
Oct 28, 2024

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Oct 21, 2024

Description

This adds the custom_playback_settings Feature Flag and the segmented tab bar in the playback effects view.

** Ignore local files for now

Project #3042

Testing Instructions

  1. Play an episode and tap effects icon in full-screen player
  2. ✅ Notice that the segmented tab bar is displayed at the top of the effects view** (hHFpI1RtnW4TRb448Q4Don-fi-4730_28640)
  3. Disable the FF
  4. Re-open the effects view and
  5. ✅ Notice that the segmented tab bar is not displayed
  6. Go to Podcasts tab
  7. Tap on locked folder icon
  8. ✅ Notice that segmented bar on the Plans page looks fine

** Only match the segmented control in Figma, the top header is highlighted only on iOS.

Screenshots or Screencast

Effects Settings

Plans

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

I have tested any UI changes...

  • with different themes
  • with a landscape orientation
  • with the device set to have a large display and font size
  • for accessibility with TalkBack

@ashiagr ashiagr added this to the Future milestone Oct 21, 2024
@ashiagr ashiagr requested a review from a team as a code owner October 21, 2024 13:44
@ashiagr ashiagr requested review from geekygecko and removed request for a team October 21, 2024 13:44
@dangermattic
Copy link
Collaborator

dangermattic commented Oct 21, 2024

1 Warning
⚠️ Class SegmentedTabBarColors is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 21, 2024

📲 You can test the changes from this Pull Request in 📱 Mobile by scanning the QR code below to install the corresponding build.
App Name 📱 Mobile
Build TypedebugProd
Commit64c14cf
Direct Downloadpocketcasts-app-prototype-build-pr3065-64c14cf.apk
📲 You can test the changes from this Pull Request in 🚗 Automotive by scanning the QR code below to install the corresponding build.
App Name 🚗 Automotive
Build TypedebugProd
Commit64c14cf
Direct Downloadpocketcasts-automotive-prototype-build-pr3065-64c14cf.apk
📲 You can test the changes from this Pull Request in ⌚ Wear by scanning the QR code below to install the corresponding build.
App Name ⌚ Wear
Build TypedebugProd
Commit64c14cf
Direct Downloadpocketcasts-wear-prototype-build-pr3065-64c14cf.apk

Copy link
Member

@geekygecko geekygecko left a comment

Choose a reason for hiding this comment

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

I have asked the design team about this change as I'm unsure if it's too much like an iOS control. It also feels quite a thin touch target. If it's ok with you I won't approve this until we have heard back from them.

Internal discussion pdeCcb-7bh-p2#comment-5776

@geekygecko geekygecko added the [Type] Feature Adding a new feature. label Oct 22, 2024
@ashiagr
Copy link
Contributor Author

ashiagr commented Oct 24, 2024

@geekygecko 👋

I've updated the component style and screenshots based on pdeCcb-7bh-p2#comment-5789.
We might need a few tweaks from the designer so I've added a do not merge label.

@ashiagr
Copy link
Contributor Author

ashiagr commented Oct 28, 2024

As the feature is behind FF, I'm merging this PR. I'll revisit it once we have further updates: pdeCcb-7bh-p2#comment-5804

@ashiagr ashiagr merged commit 8d94f87 into main Oct 28, 2024
18 of 19 checks passed
@ashiagr ashiagr deleted the task/3042-add-ff-and-control branch October 28, 2024 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants