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.

Sign up for GitHub

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

Jump to bottom

Show unread count in room's back button #781

Closed
wants to merge 5 commits into from
Closed

Show unread count in room's back button #781

wants to merge 5 commits into from

Conversation

djorkaeffalexandre
Copy link
Collaborator

@djorkaeffalexandre djorkaeffalexandre commented Apr 3, 2019 •
edited
Loading

@RocketChat/ReactNative
@diegolmello

Closes #778

Printscreen:

@codecov-io
Copy link

codecov-io commented Apr 3, 2019 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (develop@7a80550). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #781   +/-   ##
==========================================
  Coverage           ?   85.61%           
==========================================
  Files              ?       40           
  Lines              ?      702           
  Branches           ?      120           
==========================================
  Hits               ?      601           
  Misses             ?       82           
  Partials           ?       19

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a80550...16296e8. Read the comment docs.

diegolmello
diegolmello requested changes Apr 4, 2019
View reviewed changes
app/views/RoomView/index.js Outdated Show resolved Hide resolved
app/views/RoomsListView/index.js Outdated Show resolved Hide resolved
app/views/RoomsListView/index.js Outdated Show resolved Hide resolved
app/views/RoomsListView/index.js Outdated Show resolved Hide resolved
app/views/RoomsListView/index.js Outdated Show resolved Hide resolved
app/views/RoomsListView/index.js Outdated Show resolved Hide resolved
@djorkaeffalexandre
Copy link
Collaborator Author

@diegolmello I solved your reviews.

diegolmello
diegolmello requested changes Apr 17, 2019
View reviewed changes
app/views/RoomView/index.js Outdated Show resolved Hide resolved
app/views/RoomView/index.js Outdated Show resolved Hide resolved
app/views/RoomView/index.js Outdated Show resolved Hide resolved
@diegolmello
Copy link
Member

@djorkaeffalexandre Also resolve conflicts and merge develop. There's a few changes on RoomView header to make it prettier.
The title is aligned left now and you'll need to make a few layout ajustments.

@djorkaeffalexandre
Copy link
Collaborator Author

Gif with header back title:

@djorkaeffalexandre djorkaeffalexandre mentioned this pull request May 8, 2019
Closed
@djorkaeffalexandre
Copy link
Collaborator Author

Printscreen to unreadCount on Android Device:

pranavpandey1998official
pranavpandey1998official reviewed May 10, 2019
View reviewed changes
Copy link
Contributor

@pranavpandey1998official pranavpandey1998official left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It works perfectly for me, I just want to point out that in web If some channel is unread and I am not mentioned in it, it shows (.) type of indicator can you also add that

@diegolmello
Copy link
Member

It works perfectly for me, I just want to point out that in web If some channel is unread and I am not mentioned in it, it shows (.) type of indicator can you also add that

No need for the dot behaviour, but thanks for reviewing :)

diegolmello
diegolmello requested changes May 10, 2019
View reviewed changes
Copy link
Member

@diegolmello diegolmello left a comment

Choose a reason for hiding this comment

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

If you have a really long channel name, it'll cut be above right buttons and android and almost cut on iOS.

diegolmello
diegolmello requested changes May 10, 2019
View reviewed changes
Copy link
Member

@diegolmello diegolmello left a comment

Choose a reason for hiding this comment

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

Title isn't centralized vertically on landscape anymore.

image

app/views/RoomView/index.js
widthOffset={tmid ? 95 : 130}
isFetching={isFetching}
/>
headerLeft: (
Copy link
Member

Choose a reason for hiding this comment

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

Placing the entire header on the left button feels wrong and may be impacted by future react-navigation updates.

app/views/RoomView/index.js Outdated
<HeaderBackButton
title={unreadsCount > 999 ? '+999' : unreadsCount}
backTitleVisible
onPress={() => navigation.goBack()}
Copy link
Member

Choose a reason for hiding this comment

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

Can we use onPress={navigation.goBack}?

@djorkaeffalexandre djorkaeffalexandre self-assigned this May 17, 2019
@lili2311 lili2311 mentioned this pull request Nov 16, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@diegolmello diegolmello diegolmello requested changes

@pranavpandey1998official pranavpandey1998official pranavpandey1998official approved these changes

@IlarionHalushka IlarionHalushka Awaiting requested review from IlarionHalushka

@sampaiodiego sampaiodiego Awaiting requested review from sampaiodiego

Assignees

@djorkaeffalexandre djorkaeffalexandre

Labels
👍 improvement 🚨 needs changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[NEW] Show unread count in room's back button
4 participants
@djorkaeffalexandre @codecov-io @diegolmello @pranavpandey1998official