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

[HOLD] [$1000] Web - Workspace details tab leades right to left #22537

Closed
1 of 6 tasks
kbecciv opened this issue Jul 9, 2023 · 20 comments
Closed
1 of 6 tasks

[HOLD] [$1000] Web - Workspace details tab leades right to left #22537

kbecciv opened this issue Jul 9, 2023 · 20 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Jul 9, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open any workspace
  2. Click on the workspace name
  3. Click settings
  4. Click on the first option "Notify me"
  5. Click on the back arrow
  6. Click on the back arrow again

Expected Result:

The details tab should slides left to right

Actual Result:

The details tab should slides right to left, and this happens only if the user clicks on 'Notify me' option, if the user click for example on welcome message option and then clicks back arrow, the details tab slides left to right

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.38-3
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

7.New.Expensify.-.9.July.2023.mp4
Recording.3523.mp4

Expensify/Expensify Issue URL:
Issue reported by: @mejed-alkoutaini
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688909729731159

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0179249bd90ea7ad90
  • Upwork Job ID: 1678513905772654592
  • Last Price Increase: 2023-07-10
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 9, 2023

Triggered auto assignment to @maddylewis (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jul 9, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@b4s36t4
Copy link
Contributor

b4s36t4 commented Jul 9, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

Web - Workspace details tab leades right to left

What is the root cause of that problem?

RCA for this issue that how we handled routing altogether. If we observe the pattern for creating StackNavigator we placed every route related to a single section places in one navigator. Like for InitialSettings Screen all the screens are placed into one single Navigator.

But for the RoomSettings We have created two navigators ReportDetailsModalStackNavigator
ReportSettingsModalStackNavigator even though both of them are rendering on RHP

We can view the navigators here

const ReportSettingsModalStackNavigator = createModalStackNavigator([

const ReportDetailsModalStackNavigator = createModalStackNavigator([

Because of this whenever we're Moving back from NotificationScreen -> SettingsScreen Here because of the different navigator we're getting initalIndex as 0

const getActiveRouteIndex = function (route, index) {

Which is handled here ->

navigate(fallbackRoute, 'UP');
pushing the Details Screen instead of poping (going back).

If we observe the bug report, Details page is navigated 2 times.

  • One from Settings to Details
  • One from Details to Details (weird)

What changes do you think we should make in order to solve the problem?

We should merge these two navigators mentioned here.

const ReportDetailsModalStackNavigator = createModalStackNavigator([
{
getComponent: () => {
const ReportDetailsPage = require('../../../pages/ReportDetailsPage').default;
return ReportDetailsPage;
},
name: 'Report_Details_Root',
},
{
getComponent: () => {
const ShareCodePage = require('../../../pages/home/report/ReportDetailsShareCodePage').default;
return ShareCodePage;
},
name: 'Report_Details_Share_Code',
},
]);
const ReportSettingsModalStackNavigator = createModalStackNavigator([
{
getComponent: () => {
const ReportSettingsPage = require('../../../pages/settings/Report/ReportSettingsPage').default;
return ReportSettingsPage;
},
name: 'Report_Settings_Root',
},
{
getComponent: () => {
const RoomNamePage = require('../../../pages/settings/Report/RoomNamePage').default;
return RoomNamePage;
},
name: 'Report_Settings_Room_Name',
},
{
getComponent: () => {
const NotificationPreferencesPage = require('../../../pages/settings/Report/NotificationPreferencePage').default;
return NotificationPreferencesPage;
},
name: 'Report_Settings_Notification_Preferences',
},
{
getComponent: () => {
const WriteCapabilityPage = require('../../../pages/settings/Report/WriteCapabilityPage').default;
return WriteCapabilityPage;
},
name: 'Report_Settings_Write_Capability',
},
]);

Working Demo (Fixed):
op.webm

What alternative solutions did you explore? (Optional)

@syedsaroshfarrukhdot
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Web - Workspace details tab leades right to left

What is the root cause of that problem?

If we check closely details page is being rendered twice.

  • Settings to Details
  • Details to Details

As for the RoomSettings We have created two navigators ReportDetailsModalStackNavigator and ReportSettingsModalStackNavigator

So when are using goBack callback it pushes the Details Screen instead of poping because it is already in the stack.

onBackButtonPress={() => Navigation.goBack(ROUTES.getReportDetailsRoute(this.props.report.reportID))}

What changes do you think we should make in order to solve the problem?

To fix this issue instead of using goBack call back we should use the following onBackButtonPress

       Navigation.navigate(ROUTES.getReportDetailsRoute(this.props.report.reportID))}

What alternative solutions did you explore? (Optional)

@hungvu193
Copy link
Contributor

Same RCA and will be resolved here:
#21839

@maddylewis maddylewis added the External Added to denote the issue can be worked on by a contributor label Jul 10, 2023
@melvin-bot melvin-bot bot changed the title Web - Workspace details tab leades right to left [$1000] Web - Workspace details tab leades right to left Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0179249bd90ea7ad90

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

Current assignee @maddylewis is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (External)

@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

📣 @hungvu193 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

📣 @mejed! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@melvin-bot
Copy link

melvin-bot bot commented Jul 10, 2023

The BZ member will need to manually hire mejed for the Reporter role. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

@maddylewis
Copy link
Contributor

ah, i manually assigned you to this issue based on this comment - #22537 (comment)

but lmk if you don't want to work on this one, @hungvu193

@b4s36t4
Copy link
Contributor

b4s36t4 commented Jul 10, 2023

@maddylewis is this ready to pick? I think for the navigation part there's some discussion going on. Does it?

@parasharrajat
Copy link
Member

@maddylewis Was C+ unassignment intentional?

@hungvu193
Copy link
Contributor

ah, i manually assigned you to this issue based on this comment - #22537 (comment)

but lmk if you don't want to work on this one, @hungvu193

I'm happy to work on this one, but actually there were few proposals on #21839 so I think it'd better to pick proposal from that issue and put this on hold.

@melvin-bot melvin-bot bot added the Overdue label Jul 13, 2023
@maddylewis
Copy link
Contributor

okay, that makes sense to me @hungvu193. let's focus on #21839 and i will put this one on hold for now.

@melvin-bot melvin-bot bot removed the Overdue label Jul 14, 2023
@maddylewis maddylewis removed the Daily KSv2 label Jul 14, 2023
@melvin-bot melvin-bot bot removed the Overdue label Jul 14, 2023
@maddylewis maddylewis changed the title [$1000] Web - Workspace details tab leades right to left [HOLD] [$1000] Web - Workspace details tab leades right to left Jul 14, 2023
@melvin-bot melvin-bot bot added the Overdue label Jul 24, 2023
@maddylewis
Copy link
Contributor

i think we're still holding for #21839 but that issue was closed. i reopened it just to confirm #21839 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Jul 24, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 1, 2023
@maddylewis
Copy link
Contributor

holding for - #21839

@melvin-bot melvin-bot bot removed the Overdue label Aug 2, 2023
@melvin-bot melvin-bot bot added the Overdue label Aug 10, 2023
@maddylewis
Copy link
Contributor

this one (#21839) was likely fixed by this one (#23007).

@hungvu193 - what are your thoughts? is this one still outstanding? or did a proposal from #21839 end up fixing this?

@melvin-bot melvin-bot bot removed the Overdue label Aug 15, 2023
@hungvu193
Copy link
Contributor

I think this was already fixed, I couldn't reproduce it anymore, we can close 😄

Screen.Recording.2023-08-15.at.23.53.42.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests

6 participants