Text for empty chat state is shown upside down and text direction is strange

status-im / status-mobile

a free (libre) open source, mobile OS for Ethereum

https://status.app

Mozilla Public License 2.0

3.91k stars 984 forks source link

Text for empty chat state is shown upside down and text direction is strange #7915

Closed annadanchenko closed 5 years ago

annadanchenko commented 5 years ago

Description

Type: Bug

Summary: After joining public chat or 1:1 chat its text and icon are shown upside down. Only after sending there at least 1 message the text and icon are shown normally Screenshot_20190404-154848

Expected behavior

text and icon have usual position

Actual behavior

text and icon are upside down

Reproduction

Open Status
Tap +
Tap Join public chat
for topic: type "public" (or any other name of empty chat) and tap enter
check text and icon in the open empty chat

Additional Information

Status version: 0.12.0
Operating System: Android and iOS

churik commented 5 years ago

Appeared after merge https://github.com/status-im/status-react/pull/7663, but in last builds https://github.com/status-im/status-react/pull/7663#issuecomment-478569201 they were OK.

bitsikka commented 5 years ago

on it

bitsikka commented 5 years ago

found possible reason:

It is probably due to the recent migration to RN 0.59.2 which probably does not include this fix https://github.com/facebook/react-native/commit/198eb0269781803cc16254916b0477916afbcb0e

opting for temporary solution until RN is upgraded again separately.

temp fix coming up

mandrigin commented 5 years ago

@bitsikka what do you think about upgrading RN instead (0.59.2 → 0.59.3), it is just a bugfix release, it shouldn't affect our app much.

bitsikka commented 5 years ago

agreed - that's a better idea.

bitsikka commented 5 years ago

upgrade fixes the problem - tested locally
cannot send PR to nonexistent branch in status-im/react-native
PR to status-im/status-react (with only changes to package.json.orig, yarn.lock, Podfile.lock) depends on PR to status-im/react-native
These PRs for upgrading RN looks to be better performed internally by a Core dev
upgrade also fixes at least one another problem I'd noticed(password field dots leading/line-height change as you type) - probably fixes more problems

bitsikka commented 5 years ago

These PRs for upgrading RN looks to be better performed internally by a Core dev

cc @rasom ping @mandrigin

mandrigin commented 5 years ago

@bitsikka I think there is no reason for us to use our own fork or RN at that moment. all the patches we had are already in upstream. @rasom wdyt?

bitsikka commented 5 years ago

Makes sense 👍

Meanwhile, testing with mainstream RN Plus, going to keep some mind on cases/issues discussed in PR for RN upgrade to 0.59.2

bitsikka commented 5 years ago

no dice ran into issues with regular RN :(

issues like

console.error: "re-frame: no handler registered for effect:", {"ns":null,"name":"keyboard-max-height","fqn":"keyboard-max-height","_hash":-869899104,"cljs$lang$protocol_mask$partition0$":2153775105,"cljs$lang$protocol_mask$partition1$":4096}, ". Ignoring."

error
    index.bundle?platform=ios&dev=true&minify=false:66704:29
_callTimer
    index.bundle?platform=ios&dev=true&minify=false:28035:17
callTimers
    index.bundle?platform=ios&dev=true&minify=false:28242:19
__callFunction
    index.bundle?platform=ios&dev=true&minify=false:3277:49
<unknown>
    index.bundle?platform=ios&dev=true&minify=false:3034:31
__guard
    index.bundle?platform=ios&dev=true&minify=false:3231:15
callFunctionReturnFlushedQueue
    index.bundle?platform=ios&dev=true&minify=false:3033:21

mandrigin commented 5 years ago

@bitsikka so you don't have this exact issue on the latest develop? because it is for sure doesn't sound like RN-related to me.

cammellos commented 5 years ago

@bitsikka that's also in develop, https://github.com/status-im/status-react/issues/7917 has the same logs

It means some code is returning db instead of cofx somewhere, I can take a look at it on Monday.

bitsikka commented 5 years ago

@cammellos my bad.. I know at least one place in the code I wrote where I'm returning db instead of cofx. I'm testing it out and so far it gets rid of that particular error above, but, emits another similar error having to do with wallet. Going to continue hunt down and testing.

Thanks for that insight too - I was baffled by the error but now it makes sense.

bitsikka commented 5 years ago

@cammellos, @mandrigin fixed (in PR #7922 as a fix for #7917) the error I mentioned above

So, that error is indeed not concerned with this issue

Hence, no reason so far to not make a PR to upgrade RN to 0.59.3, which will fix this issue

mandrigin commented 5 years ago

great! ping me when it's ready!

bitsikka commented 5 years ago

great! ping me when it's ready!

ping @mandrigin