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

fix: disable typing events when offline #2753

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

isekovanic
Copy link
Contributor

🎯 Goal

Solves the typing errors mentioned in this GH issue.

🛠 Implementation details

🎨 UI Changes

iOS
Before After
Android
Before After

🧪 Testing

☑️ Checklist

  • I have signed the Stream CLA (required)
  • PR targets the develop branch
  • Documentation is updated
  • New code is tested in main example apps, including all possible scenarios
    • SampleApp iOS and Android
    • Expo iOS and Android

@Stream-SDK-Bot
Copy link
Contributor

Stream-SDK-Bot commented Nov 4, 2024

SDK Size

title develop branch diff status
js_bundle_size 444.8212890625 KB 451 KB +6151 B 🔴

@isekovanic
Copy link
Contributor Author

SDK Size

title develop branch diff status
js_bundle_size 444.8212890625 KB 451 KB +6151 B 🔴

Yeah this is 100% broken @khushal87 & @oliverlaz :D I think it interchangeably gets confused between our v6 branch and anything that goes to develop, since:

  • This PR was just fine, without any issues
  • Then, this PR was merged and it reduced the SDK size by 6KB
  • Then finally this one was opened only to fail.

@oliverlaz
Copy link
Member

SDK Size

title develop branch diff status
js_bundle_size 444.8212890625 KB 451 KB +6151 B 🔴

Yeah this is 100% broken @khushal87 & @oliverlaz :D I think it interchangeably gets confused between our v6 branch and anything that goes to develop, since:

  • This PR was just fine, without any issues
  • Then, this PR was merged and it reduced the SDK size by 6KB
  • Then finally this one was opened only to fail.

hey @testableapple can you please shed some light here? How does this calculation work under the hood? Do we have some caches that are messing up the reporting?

@testableapple
Copy link
Contributor

hey @testableapple can you please shed some light here? How does this calculation work under the hood? Do we have some caches that are messing up the reporting?

Hey @oliverlaz, well, there is no caching. There is only one json file that is being updated on every merge to develop and main accordingly.

So every PR is measured and compared with the develop value in the json file above.

In regards to this issue, @isekovanic is right, the SDK Size action gets confused because of the push to v6 branch (it only expects pushes to develop and main, which is an issue I guess). I'll fix this and open a PR in this repo shortly.

@isekovanic
Copy link
Contributor Author

Thanks so much @testableapple ! Do you have any advice for how to approach this whenever we do branch builds ? We sometimes need to do this in order to test on Release versions of our sample apps.

@testableapple
Copy link
Contributor

@isekovanic, here is the fix - #2755
When it's merged, please keep an eye on the SDK Size action in your next PR.

Do you have any advice for how to approach this whenever we do branch builds?

Hmm, I don't see how this might be related 🤔
As long as you do not push to develop or main directly, it should not be a problem.

@isekovanic
Copy link
Contributor Author

@isekovanic, here is the fix - #2755 When it's merged, please keep an eye on the SDK Size action in your next PR.

Do you have any advice for how to approach this whenever we do branch builds?

Hmm, I don't see how this might be related 🤔 As long as you do not push to develop or main directly, it should not be a problem.

Ah, I misunderstood what you were saying - my bad. Ignore the last inquiry about branch builds 👍

@isekovanic isekovanic merged commit ba9bccd into develop Nov 4, 2024
6 of 7 checks passed
@isekovanic isekovanic deleted the fix/disable-typing-events-when-offline branch November 4, 2024 15:13
@github-actions github-actions bot mentioned this pull request Nov 4, 2024
6 tasks
@stream-ci-bot
Copy link
Contributor

🎉 This PR is included in version 5.41.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants