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.

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

Rename profile page to details page #1931

Merged
merged 3 commits into from
Mar 23, 2021
Merged

Conversation

NikkiWines
Copy link
Contributor

Details

Currently in E.cash there are two profile pages: The page you get when you click on the chat header avatar of a chat you have with someone, and the page you get when navigating to Settings > Profile.

The non-settings profile page displays a header of Details while the settings profile page displays Profile.

There's no reason for these pages to be named the same thing, and from the description above you can see how much more difficult to describe which page is which.

Fixed Issues

Fixes N/A, was brought up in conversation and is an easy fix.

Tests

  1. Log into E.cash
  2. Create a chat with one person
  3. Click on their chat header avatar and confirm you can see the Details page for that user.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

Screen Shot 2021-03-19 at 10 59 21 AM

Mobile Web

Desktop

Screen Shot 2021-03-19 at 11 00 15 AM

iOS

Android

@NikkiWines NikkiWines requested a review from a team as a code owner March 19, 2021 18:23
@NikkiWines NikkiWines self-assigned this Mar 19, 2021
@botify botify requested review from jasperhuangg and removed request for a team March 19, 2021 18:23
Copy link
Contributor

@jasperhuangg jasperhuangg left a comment

Choose a reason for hiding this comment

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

@NikkiWines Looks good! Merge conflicts just need to be solved and then it should be good to go!

@@ -63,7 +63,7 @@ const HeaderView = props => (
onPress={() => {
const {participants} = props.report;
if (participants.length === 1) {
Navigation.navigate(ROUTES.getProfileRoute(participants[0]));
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this is kinda unrelated to your changes, but it looks like participants is a duplicate name from the upper scope (line 53). Is there any reason we're re-accessing/redefining a variable for props.reports.participants here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right that there's no need to redefine this here and actually it convenient got changed/resolved after merging master.

@NikkiWines
Copy link
Contributor Author

Updated and resolved conflicts

@jasperhuangg jasperhuangg merged commit 7b0342a into master Mar 23, 2021
@jasperhuangg jasperhuangg deleted the nikki-rename-profilePage branch March 23, 2021 05:29
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants