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: implementation toDataURL for iOS platform both architectures #2405

Merged
merged 3 commits into from
Aug 13, 2024

Conversation

bohdanprog
Copy link
Member

@bohdanprog bohdanprog commented Aug 12, 2024

Closes #2233

Summary

Just wanted to say a big thank you to @imanderson for sorting out that issue with the toDataURL method in iOS and the old Arch. It’s awesome how you tracked down the problem and jumped in with a fix.

Problem: in iOS and old Arch, a callback on toDataURL method will only be called the second time of asking.
Here’s my PR based on his changes: PR

Test Plan

We can easily test that fix by running the Test2233, we have an example of the problem.

What are the steps to reproduce (after prerequisites)?

Compatibility

OS Implemented
iOS

@bohdanprog bohdanprog requested a review from jakex7 August 12, 2024 15:17
@bohdanprog bohdanprog self-assigned this Aug 12, 2024
@bohdanprog bohdanprog marked this pull request as ready for review August 12, 2024 15:56
Copy link
Member

@jakex7 jakex7 left a comment

Choose a reason for hiding this comment

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

Left one comment, other than that LGTM

apple/RNSVGSvgViewModule.mm Outdated Show resolved Hide resolved
@jakex7 jakex7 merged commit 3350ab4 into main Aug 13, 2024
5 checks passed
@jakex7 jakex7 deleted the fix/toDataUrl-ios-old-arch branch August 13, 2024 08:45
github-merge-queue bot referenced this pull request in valora-inc/wallet Sep 24, 2024
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[react-native-svg](https://github.com/react-native-community/react-native-svg)
| [`^15.5.0` ->
`^15.7.1`](https://renovatebot.com/diffs/npm/react-native-svg/15.5.0/15.7.1)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/react-native-svg/15.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/react-native-svg/15.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/react-native-svg/15.5.0/15.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/react-native-svg/15.5.0/15.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>react-native-community/react-native-svg
(react-native-svg)</summary>

###
[`v15.7.1`](https://github.com/software-mansion/react-native-svg/releases/tag/v15.7.1)

[Compare
Source](https://github.com/react-native-community/react-native-svg/compare/v15.7.0...v15.7.1)

Hotfix for v15.7.0

#### What's Changed

- fix: exclude e2e from types in release by
[@&#8203;jakex7](https://github.com/jakex7) in
#[https://github.com/software-mansion/react-native-svg/pull/2454](https://github.com/software-mansion/react-native-svg/pull/2454)

**Full Changelog**:
software-mansion/react-native-svg@v15.7.0...v15.7.1

###
[`v15.7.0`](https://github.com/software-mansion/react-native-svg/releases/tag/v15.7.0)

[Compare
Source](https://github.com/react-native-community/react-native-svg/compare/v15.6.0...v15.7.0)

A minor release introducing the implementation of filter regions along
with some bug fixes 🔧

#### What's Changed

- fix: android svg scale 0 by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2424](https://github.com/software-mansion/react-native-svg/pull/2424)
- feat: e2e snapshot tests by
[@&#8203;CDFN](https://github.com/CDFN) in
[https://github.com/software-mansion/react-native-svg/pull/2338](https://github.com/software-mansion/react-native-svg/pull/2338)
- feat: add load method by
[@&#8203;WoLewicki](https://github.com/WoLewicki) in
[https://github.com/software-mansion/react-native-svg/pull/2427](https://github.com/software-mansion/react-native-svg/pull/2427)
- fix: reset paint settings before drawing final bitmap by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2439](https://github.com/software-mansion/react-native-svg/pull/2439)
- fix: add deprecated SvgViewManager to not break 0.72 by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2435](https://github.com/software-mansion/react-native-svg/pull/2435)
- docs: introduce about webpack by
[@&#8203;bohdanprog](https://github.com/bohdanprog) in
[https://github.com/software-mansion/react-native-svg/pull/2434](https://github.com/software-mansion/react-native-svg/pull/2434)
- docs: broken link in the readme file by
[@&#8203;JohnAdib](https://github.com/JohnAdib) in
[https://github.com/software-mansion/react-native-svg/pull/2443](https://github.com/software-mansion/react-native-svg/pull/2443)
- feat: implement filter region by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2441](https://github.com/software-mansion/react-native-svg/pull/2441)
- fix: Android group `opacity` prop by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2417](https://github.com/software-mansion/react-native-svg/pull/2417)
- fix: render G offscreen only when it's needed (opacity != 1) by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2450](https://github.com/software-mansion/react-native-svg/pull/2450)
- fix: transform scale on android by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2452](https://github.com/software-mansion/react-native-svg/pull/2452)

#### New Contributors

- [@&#8203;JohnAdib](https://github.com/JohnAdib) made their
first contribution in
[https://github.com/software-mansion/react-native-svg/pull/2443](https://github.com/software-mansion/react-native-svg/pull/2443)

**Full Changelog**:
software-mansion/react-native-svg@v15.6.0...v15.7.0

###
[`v15.6.0`](https://github.com/software-mansion/react-native-svg/releases/tag/v15.6.0)

[Compare
Source](https://github.com/react-native-community/react-native-svg/compare/v15.5.0...v15.6.0)

In this version, we've introduced support for React Native 0.75, Fabric
support on Windows, and a few minor enhancements and fixes. Thank you to
everyone who contributed! 🚀

#### What's Changed

- chore: bump reanimated in macos example on paper by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2398](https://github.com/software-mansion/react-native-svg/pull/2398)
- feat: add support int32 color by
[@&#8203;bohdanprog](https://github.com/bohdanprog) in
[https://github.com/software-mansion/react-native-svg/pull/2397](https://github.com/software-mansion/react-native-svg/pull/2397)
- fix: implementation toDataURL for iOS platform both architectures by
[@&#8203;bohdanprog](https://github.com/bohdanprog) in
[https://github.com/software-mansion/react-native-svg/pull/2405](https://github.com/software-mansion/react-native-svg/pull/2405)
- chore: cleanup example apps by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2408](https://github.com/software-mansion/react-native-svg/pull/2408)
- feat(windows): add fabric support by
[@&#8203;marlenecota](https://github.com/marlenecota) in
[https://github.com/software-mansion/react-native-svg/pull/2321](https://github.com/software-mansion/react-native-svg/pull/2321)
- feat: support dataUri for android platform by
[@&#8203;bohdanprog](https://github.com/bohdanprog) in
[https://github.com/software-mansion/react-native-svg/pull/2396](https://github.com/software-mansion/react-native-svg/pull/2396)
- feat: introduce hitSlop prop by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2407](https://github.com/software-mansion/react-native-svg/pull/2407)
- feat: rewrite `Svg` transform by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2403](https://github.com/software-mansion/react-native-svg/pull/2403)
- Support-rgb(R%,G%,B%)-percentage by
[@&#8203;hryhoriiK97](https://github.com/hryhoriiK97) in
[https://github.com/software-mansion/react-native-svg/pull/2363](https://github.com/software-mansion/react-native-svg/pull/2363)
- chore: bump apps to RN 0.75 by
[@&#8203;WoLewicki](https://github.com/WoLewicki) in
[https://github.com/software-mansion/react-native-svg/pull/2340](https://github.com/software-mansion/react-native-svg/pull/2340)
- chore(deps): bump rexml from 3.2.6 to 3.3.3 in /tests-example by
[@&#8203;dependabot](https://github.com/dependabot) in
[https://github.com/software-mansion/react-native-svg/pull/2410](https://github.com/software-mansion/react-native-svg/pull/2410)
- chore: fix prettier by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2416](https://github.com/software-mansion/react-native-svg/pull/2416)
- fix: do not resolve asset url for every object on web. by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2419](https://github.com/software-mansion/react-native-svg/pull/2419)
- fix: transforms on macOS old arch by
[@&#8203;jakex7](https://github.com/jakex7) in
[https://github.com/software-mansion/react-native-svg/pull/2420](https://github.com/software-mansion/react-native-svg/pull/2420)

#### New Contributors

- [@&#8203;hryhoriiK97](https://github.com/hryhoriiK97) made
their first contribution in
[https://github.com/software-mansion/react-native-svg/pull/2363](https://github.com/software-mansion/react-native-svg/pull/2363)

**Full Changelog**:
software-mansion/react-native-svg@v15.5.0...v15.6.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 5pm,every weekend" in timezone
America/Los_Angeles, Automerge - "after 5pm,every weekend" in timezone
America/Los_Angeles.

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/valora-inc/wallet).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC44MC4wIiwidXBkYXRlZEluVmVyIjoiMzguODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsibnBtIiwicmVub3ZhdGUiXX0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: valora-bot <valorabot@valoraapp.com>
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.

toDataURL only works the second time it is executed in old Arch
2 participants