[8069] feature - [Profile] My profile edit and share screens #8722
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (120)
|
Why not tho? |
|
Tests are failing with weird: I've seen it on other PRs, no idea what it's about. Maybe @antdanchenko or @churik can help us out. |
Because I thought it'd only change the bit about Anyway.. to see why the builds are failing I commented out then.. I rebased to latest develop.. builds are still failing :( cannot see the logs Now I'm switching to Android dev build in the hope of identifying build problems.. and I need to test in Android anyway as well |
That's possible that it's not sorted, I'll check that out. Thanks. |
|
@bitsikka tests are broken in develop, sorry, we'll fix it soon |
when I used plain old then later I'll re-enable |
|
@bitsikka fixed. please rebase |
|
I checked running |
|
ok
in a little bit |
re-generating
is taking a while in my machine went for a break and came back.. still going (watching Should I just push without the re-generated files for now? I still have to do the reviewed items - which will be next btw thanks always for a prompt review 👍 |
|
regeneration done! just pushed 🤞 I have not tested at all after rebase and stuff.. testing iOS only first [still problems with android build] |
|
anyway.. I'll be testing and trying to wrap remaining known tasks today |
73% of end-end tests have passedFailed tests (12)Click to expand
Passed tests (32)Click to expand |
|
Fixed reviewed items Will test Android tomorrow |
Failing tests totally expected because there is no longer It moved to toolbar action button area |
On IOS still the same :( |
|
ok then as a work around until it is possible to test |
|
this one works fine! |
|
now that I think of it based on @errorists comment a number of comments above that said
I think following would have worked as well keep animating lesson learned
|
|
truly thank you all for the patience 😅 and baring with me on this |
|
Checked briefly on:
@bitsikka please hold on with merging, seems there is an issue with profile icons on Android 6. Rechecking on real devices. |
|
@errorists for IPhones older than X, additional shadow is displayed on scrolling. Can we live with it? |
@churik that can probably be fixable.. so I just pushed a fix
please take you time.. this PR has quite many changes, and so the chance of something going wrong is there
|
|
@bitsikka IOS issues are fixed 👍 It is only in this PR(checked |
@churik I have made a change which I think should fix it but, I do not have those phones so I tried "Nexus 6 Android 6" AVD, I tried couple of others too the app wouldn't even start in them :( please test in real devices and let me know how it goes
|
|
@bitsikka checked. |
Could you tell me if it is only on that screen or other screens inside too? |
|
@bitsikka ofc! |
Now I see why resolution was kept low 😅 react-native doc for
what is the course of action here? @errorists @flexsurfer @rachelhamlin size is 256 at the moment, should we change it back to 40 and live with blurry in bigger sizes? |
|
So first of all a huge thanks for getting all of this done man! As for the blurry identicons, I’m ok with moving that to a separate PR starting now, because it is a problem. It’s a small dumb image made of two colors, not an Instagram photo feed... we’re keeping it sharp at 256, if some devices can’t keep up, can’t we provide a fallback or conditional? Also, you can try 128 because even @2x will be a significant improvement over this blurry mess. |
you're most welcome! :)
I hear you, I agree. This has got to be addressed (but in a separate PR). It is only few old Android phones as far as I can tell (at least, now we know; hope the list of problematic Androids are documented somewhere @churik :). Surely we can isolate those and handle them separately. As for now, I realized that, before this PR, the Photo for profile from-gallery/captured-from-camera was 150, identicon was 40. So, I made all 150. Theoretically, it should work. Hope it does. |
|
@bitsikka |
Signed-off-by: Andrey Shovkoplyas <[email protected]>
Fixes #8069
Review notes
make nix-update-leinbecause of adding this, but did not include the changes it made to:Testing notes
Other notes
status: ready