-
Notifications
You must be signed in to change notification settings - Fork 714
Integrate the new SDK to support mini program #526
base: master
Are you sure you want to change the base?
Conversation
大佬喝茶 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@little-snow-fox 能保持与原来的接口兼容吗?这样会导致之前的应用都需要重写,多谢!
能否只是在原代码基础上新增:升级 SDK、分享小程序以及跳转小程序,这样的话 PR 会更容易被合并进来。 |
确实,方法名称与用法的变更对已使用该库的用户影响相当大,这也是我迟迟不肯推送合并的原因。 |
如果你愿意,我可以抽时间在你的基础上做修改,不过需要你把我添加为你仓库的 collaborator。
…On Mon, Oct 14, 2019, 8:28 PM snowfox ***@***.***> wrote:
确实方法名称与用法的变更对已使用该库的用户影响相当大,这也是我迟迟不肯推送合并的原因。
为了与微信官方文档用法保持一致,而没沿用该库原来的方法名称和用法,确实是我考虑不周,这需要专门花时间来处理。
谢谢你抽出宝贵的时间来审查代码~
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#526?email_source=notifications&email_token=AAOYTF4YUTVSKZGAWAVBDZTQORQXNA5CNFSM4JAHSY32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBEONUA#issuecomment-541648592>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOYTF2QFI3GSOTH7JQ6TB3QORQXNANCNFSM4JAHSY3Q>
.
|
@yorkie 太好了,很高兴你能够参与。 |
@little-snow-fox @M1sery 我基于这个 PR 的基础,做了一些改动(307ed4e)主要如下:
请两位帮忙一起看看,如果没有什么问题,iOS 部分我将在这个 PR 内继续完成。 |
我也添加到了 major 标签,这个 PR 合并后,Release 版本需要变更为:2.0.0
|
@yorkie 关于被微信移除掉的 registerAppWithDescription 方法,我想提出一个兼容性方案,为了不影响正在使用本库的用户,我们可以保留 registerAppWithDescription 接口,然后把它指向 SDK 的 registerApp 方法,并且给出一个 console.warn 警告用户它已经被移除掉。你觉得这样做可以吗? |
我觉得没问题,不过我记得 Node.js 有专用的 deprecation 方法,不确定 ReactNative 是否也有类似方法。 |
@yorkie 请问你这边修改 JavaScript 接口代码的时候,有修改过 Object-c 和 Java 方面的代码吗?我似乎没看到这方面的修改记录。 |
Android 部分的代码有修改过,做了一些简单的抽象,iOS 的暂时没做。
…On Wed, Oct 16, 2019, 9:26 AM snowfox ***@***.***> wrote:
@yorkie <https://github.com/yorkie> 请问你这边修改 JavaScript 接口代码的时候,有修改过
Object-c 和 Java 方面的代码吗?我似乎没看到这方面的修改记录。
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#526?email_source=notifications&email_token=AAOYTFYRUFB4L6PHJQCSGLTQOZUVLA5CNFSM4JAHSY32YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBKXB6Y#issuecomment-542470395>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOYTFYF45DQQONFDOLWPYDQOZUVLANCNFSM4JAHSY3Q>
.
|
@yorkie 如果只是修改了 JavaScript 层面的代码,这个库将没法跑起来,因为核心的地方是 object-c 和 java 方面的代码, JavaScript 层的接口方法修改后需要到 IOS 和 Android 目录实现相应功能。。。 |
|
@yorkie 但是我似乎找不到 Object-c 的改动。 |
嗯,iOS 的部分我稍后会加上,我们先基于 Android 和 JS 代码的基础来讨论对外的接口,以及浇水层接口的方式是否合适 :) |
builder = builder.setResizeOptions(resizeOptions); | ||
} | ||
// if (resizeOptions != null) { | ||
// builder = builder.setResizeOptions(resizeOptions); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@little-snow-fox 为什么我们这里要抛弃掉原来使用的 setResizeOptions
而自己实现,我对 Android 不是很熟,所以想问一下。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
因为这个方法只是对图片进行分辨率大小裁剪而已,并不是压缩大小,无法保证裁剪出来后的图片大小不超过32kb。
ios/README.txt
Outdated
@@ -0,0 +1,114 @@ | |||
重要! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个 README.txt 是否可以删除,微信 SDK 的更新日志放在这感觉没有必要。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以的,但我希望有个地方可以记录当前的 SDK 版本号。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
可以的,但我希望有个地方可以记录当前的 SDK 版本号。
根目录的 README.md 里有相应的 SDK 版本的。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好的,明白。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个问题在之后 iOS 修改后解决吧,现在大家先集中 Review 下 Android + JS 层的代码吧。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@little-snow-fox @yorkie 测试 Android 部分代码时发现两个问题,帮忙再 review 一下。
} | ||
|
||
if (imgUrl == null) { | ||
callback.invoke(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
callback.invoke(null); | |
callback.invoke(new Object[]{}); |
@yorkie 这里对于非 varargs 的调用, 使用 Object[], 不然编译报错。
imgUrl = Uri.parse(data.getString("imageUrl")); | ||
if (imgUrl.getScheme() == null) { | ||
// handle static resource if no schema is provided. | ||
imgUrl = getResourceDrawableURI(getReactApplicationContext(), imgUrl); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yorkie 看一下这块代码,这里 imgUrl 是 Uri 类型,再转换为 String 类型编译会报错。
* fix: Bitmap to byte[] compile error
* fix: remove spaces and format code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@little-snow-fox @yorkie 这里我顺便修改了一下 bitmapTopBytes 方法。=> fix commit
…at into refactor-and-upgrade
最新的提交里修改了一些 iOS 上的错误,不过还有一些库依赖问题。 |
* fix: format comments
message.messageExt = data.getString("messageExt"); | ||
} | ||
if (thumb != null) { | ||
if (thumb.length() / 1024 > THUMB_SIZE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里直接使用 thumb.length()
拿不到 thumb 的大小,我明天再提个 pr 修复这个问题。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private static byte[] bitmapTopBytes(Bitmap bitmap, final boolean needRecycle) { }
这里方法名 bitmapTopBytes 拼错了,准确的命名应该是 bitmapToBytes 。
* fix: rename bitmapTopBytes => bitmapToBytesArray
大佬们,什么时候能支持跳转小程序啊 |
@little-snow-fox @yorkie 有空找个时间一起把这个 pr 的改动再捋一遍,列个 TODO list,测试完看能不能更一版。 |
等了很久的这个PR了,大佬们辛苦了 👍 |
大佬们辛苦了, 我们想更新这个版本,可以的话,我们可以在项目中先更新这个PR方便测试发现问题。 |
时刻关注这个PR,感谢大佬们的贡献 |
还在进行吗? |
我项目都写完几个了,这个pr居然还没合并 |
@gdoudeng 感觉有点难了,我直接用 little-snow-fox/react-native-wechat-lib 了 |
我觉得既然微信SDK都大版本更新了,这边也没必要一定要保持和之前接口的兼容,愿意更新到大版本的需要接受breaking change |
合理,新项目用它完全可以 |
这个pr流产了吗?伤心... |
我打算搞下来自己维护了。。。伤心。。。 |
还处理吗? |
还在维护吗? |
使用全新的 wechat SDK,更换掉目前年份已久的SDK。
重写大部分接口,包含微信登陆、注册SDK、分享文本、分享图片、分享视频、分享网页、分享小程序、跳转到小程序等接口,并且以上接口在 IOS 和 Android 下通过简单的测试。
由于本次更新核心代码几乎重写,所以请谨慎合并,避免影响正在使用该库的用户,特别是他们的生产环境。