Skip to content

Conversation

@wreiske
Copy link
Contributor

@wreiske wreiske commented Sep 5, 2019

@RocketChat/ReactNative

This is a work in progress. Any help is greatly appreciated!

Here are some sample images so far:

636Amfvuo8

Screenshot_1567669035

explorer_S985cdk1ke
Screenshot_1567668896
Screenshot_1567667858

Screenshot_1567670521
Screenshot_1567670517
Screenshot_1567670627

Screenshot_1567671641

@lgtm-com
Copy link

lgtm-com bot commented Sep 5, 2019

This pull request introduces 1 alert when merging 61de50c into 0ea0dc2 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@wreiske
Copy link
Contributor Author

wreiske commented Sep 5, 2019

Things that need to be figured out:

  • It doesn't look like the 'user-status' events are updating / or there's something needed to redraw the statuses. They aren't changing if you change the status on another client.
  • Figure out why the status isn't pulling in on the profile page to edit
  • The status is currently showing as the current user in every channel. Need to figure out how to show the room user's status instead of the current logged in user.

Should I use the header background color here?
image
image

Instead of the custom color I picked above? Maybe there's a better color to put there....

@lgtm-com
Copy link

lgtm-com bot commented Sep 5, 2019

This pull request introduces 1 alert when merging f90b76f into 0ea0dc2 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

return (
<SafeAreaView style={styles.container} testID='room-view' forceInset={{ vertical: 'never' }}>
<StatusBar />
{this.t === 'd' && user && user.statusText ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

create a function like a renderStatusText() :)

@wreiske
Copy link
Contributor Author

wreiske commented Sep 9, 2019

I have a strict deadline that I must hit this week, so it will be later in the week before I will have time to mess with this again. If anyone wants to take a stab at it, go ahead. It'll be Thursday or Friday before I go at it again. Thanks!

@Prateek93a
Copy link
Contributor

@wreiske @diegolmello @djorkaeffalexandre Hello, I would like to show my work on this feature. I used all the code that @wreiske had written to implement the above mentioned feature. Most of the work was already done. I added my code to solve the 3 issues mentioned above, added change status modal and added theme support and resolved the conflicts. Also I thought it would be better not to show status text at the top of the room view, so I removed it but it can be added again if needed. The gif is shown below. Does it look fine?

ezgif com-crop(14)

@wreiske
Copy link
Contributor Author

wreiske commented Jan 20, 2020

@Prateek93a Thanks for updating and fixing the issues described above. Can you open a pull request to merge into wreiske:custom-status-ui? I will look at the changes and merge them into this PR.

I would rather (IMO) see the status on the room view if it's a direct message, there's room there to have it, so why not show it? People aren't going to click into user's profiles to see their status -- but they will see it if it's in the room header.

@Prateek93a
Copy link
Contributor

@wreiske Yes you are right. Now I get it, status should be visible on the room view as well so that people can see it otherwise not everyone will look into the room info to see status. Thanks.
Also,I am a little confused here, how should I raise Pr to merge into wreiske:custom-status-ui? Should I raise Pr in your forked repo?

@wreiske
Copy link
Contributor Author

wreiske commented Jan 20, 2020

That would work, yes. I will review it in the next 2 business days and get it merged in. That will update this PR.

@Prateek93a
Copy link
Contributor

Prateek93a commented Jan 20, 2020

I tried to do that. First I tried to fork your repo but github gave message that I already had a forked repo of Rocket.Chat.ReactNative.Then I tried to directly raise Pr to your forked repo, but that gave access denied error.

@wreiske
Copy link
Contributor Author

wreiske commented Jan 20, 2020

I created a PR. It should automatically update as you push changes to your branch.

wreiske#1

@Prateek93a
Copy link
Contributor

Ok Thanks

@wreiske
Copy link
Contributor Author

wreiske commented Jan 25, 2020

@Prateek93a if you can take a look at why the status modal isn't refreshing the latest status, that'd be great. thanks!

  • Change status on mobile app.
  • Change status on desktop
  • Tap to change status on mobile app to see the original status message in the modal.

Screen Shot 2020-01-25 at 2 55 41 PM

@Prateek93a
Copy link
Contributor

@wreiske Yes, surely. I will try to fix by tomorrow .

@wreiske
Copy link
Contributor Author

wreiske commented Jan 25, 2020

@wreiske Yes, surely. I will try to fix by tomorrow .

Also, the original bugs still exist... The status message does not refresh in the mobile app when changed on desktop. You have to click out of the room and click back in to see the change.

image

It also looks like this introduced some bugs. When there is no status message, the status bar shouldn't be displayed in a DM.
Screen Shot 2020-01-25 at 3 07 44 PM

The status also doesn't refresh in the user profile:
image

image

Thanks for all the help!

@Prateek93a
Copy link
Contributor

Ok I will look into the bug, and also make the changes that you suggested.

@Prateek93a
Copy link
Contributor

@Prateek93a if you can take a look at why the status modal isn't refreshing the latest status, that'd be great. thanks!

* Change status on mobile app.

* Change status on desktop

* Tap to change status on mobile app to see the original status message in the modal.
Screen Shot 2020-01-25 at 2 55 41 PM

@wreiske I have fixed the above bug, and also added the code to hide the status text bar when there is no status. Now I am working on the last bug where the room status was not getting updated. Also I tried pushing my commits, but for that I had to pull the changes first. But when I pulled, the code was still having duplicates, and fixed color. I am not sure why this happened. So shall I remove them and then push the commits?

@wreiske
Copy link
Contributor Author

wreiske commented Jan 26, 2020

It may be best to fork my repo, edit the custom status ui branch, and then make a pull request back. If you fork my repo on the branch this PR is, it shouldn't have any duplicates. That way you run another PR to my fork, I'll review and test, and then merge it and this PR will update.

@Prateek93a
Copy link
Contributor

Actually I tried to do that but since I already have a forked this repo, so I am not able to fork your repo.
I have received the latest commit you made i.e. merge pull request but in that, there was no commit to remove the duplicate code. I think if you push the remove duplicates commit again, then after pulling the changes I shall receive those changes.

@djorkaeffalexandre djorkaeffalexandre mentioned this pull request Mar 2, 2020
6 tasks
@diegolmello diegolmello closed this Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants