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

chore: Add flex_zone pages to Mercury screen requests #2317

Merged
merged 3 commits into from
Dec 2, 2024

Conversation

cmaddox5
Copy link
Contributor

Mercury asked for us to append all flex zone pages to each data request. This will allow them to make less requests and improve battery life. Because the layout is required to get the flex zone data, I decided to make an alternative to get/2 that works through select_variant/3 with a different then_fn. I worry this might be a tad overcomplicated so happy to hear alternatives.

@cmaddox5 cmaddox5 requested a review from a team as a code owner November 25, 2024 18:52
Copy link
Contributor

@digitalcora digitalcora left a comment

Choose a reason for hiding this comment

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

Conditionally approving: I think we should have a controller test for this, to verify that A) it works as intended right now and B) we don't break Mercury screens in the future, since that kind of thing might otherwise slip past us in our own manual testing. It doesn't need a ton of detail, just that the flex_zone exists under the right circumstances.

lib/screens_web/controllers/v2/screen_api_controller.ex Outdated Show resolved Hide resolved
@cmaddox5 cmaddox5 force-pushed the cm/add-flex-zone-to-data-response branch from b64dd5a to 140e2e9 Compare December 2, 2024 16:22
@cmaddox5 cmaddox5 force-pushed the cm/add-flex-zone-to-data-response branch from 140e2e9 to 8479f82 Compare December 2, 2024 16:27
@cmaddox5 cmaddox5 merged commit e6a2a20 into main Dec 2, 2024
11 checks passed
@cmaddox5 cmaddox5 deleted the cm/add-flex-zone-to-data-response branch December 2, 2024 17:47
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