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

Restored 3-input constructor for SessionWrapper (to preserve mobile b… #1446

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

OrangeAndGreen
Copy link
Contributor

Restored 3-input constructor for SessionWrapper (to preserve mobile build).

This change is to fix the mobile build, after a recent commit changed a constructor for SessionWrapper to take a new field called windowWidth. This field is only stored by the wrapper for later external access, and is never used by the mobile code. So I added a new 3-input constructor that then passes null for the windowWidth.

@avazirna
Copy link
Contributor

avazirna commented Oct 18, 2024

@OrangeAndGreen it might be better to lean on the consistency side, why not pass the window width on mobile too instead of setting it to null? It sounds that there is no use for it, but considering that the session will now carry this property we might want to set an actual value.

Copy link
Contributor

@shubham1g5 shubham1g5 left a comment

Choose a reason for hiding this comment

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

Added an issue to pass window width - dimagi/commcare-android#2878 as a separate item, and going to merge this to get the master builds passing. In general, we should be treating the master build failures as a critical event and should be fixing those promptly as it affects other external and internal devs working on the project.

@shubham1g5 shubham1g5 merged commit 7cc0932 into master Oct 21, 2024
2 checks passed
@shubham1g5 shubham1g5 deleted the dv/session_wrapper_mobile_fix branch October 21, 2024 12:05
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.

3 participants