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] [$2000] mWeb - Chat header goes out of the view after opening keyboard #13491

Closed
kavimuru opened this issue Dec 10, 2022 · 97 comments
Closed
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Dec 10, 2022

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Tap on fab menu> New Chat
  2. Search a new contact
  3. Open keyboard

Expected Result:

The chat header should be visible

Actual Result:

The chat header goes out of the view

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.2.37-1
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

video_20221209_175140_edit.mp4
az_recorder_20221209_185441.1.mp4

Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1670588965535099

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01386943f03f5f8c21
  • Upwork Job ID: 1602394546228416512
  • Last Price Increase: 2023-01-24
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 10, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 10, 2022

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

@esh-g
Copy link
Contributor

esh-g commented Dec 11, 2022

Is this intended behaviour or a bug?

@melvin-bot
Copy link

melvin-bot bot commented Dec 12, 2022

Triggered auto assignment to @pecanoro (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@mallenexpensify
Copy link
Contributor

@kavimuru just getting back after a month off, are we not putting the issue reporter in the title anymore?

Issue reported by: @thesahindia

I was unable to reproduce the issue on mobile Safari (iOS), I was able to reproduce on mobile Chrome though.

@pecanoro I'm pretty sure this can be external but would love bonus 👀 from you to be sure. Thanks. (also.. if any process has changed the past month for how we're managing issues, let me know)

RPReplay_Final1670873402.MP4

@melvin-bot melvin-bot bot removed the Overdue label Dec 12, 2022
@pecanoro
Copy link
Contributor

Yes, it can be external! Adding the label! Behaviour should be consistent in all platforms.

@pecanoro pecanoro added the External Added to denote the issue can be worked on by a contributor label Dec 12, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 12, 2022

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

@melvin-bot melvin-bot bot changed the title mWeb - Chat header goes out of the view after opening keyboard [$1000] mWeb - Chat header goes out of the view after opening keyboard Dec 12, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 12, 2022

Job added to Upwork: https://www.upwork.com/jobs/~01386943f03f5f8c21

@melvin-bot
Copy link

melvin-bot bot commented Dec 12, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 12, 2022
@melvin-bot
Copy link

melvin-bot bot commented Dec 12, 2022

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

@sandipanghos
Copy link

@mananjadhav
mananjadhav
@pecanoro
pecanoro
@mallenexpensify
mallenexpensify

Can i post the proposal for this issue. Is it still open?

@mananjadhav
Copy link
Collaborator

Yes it is still open for proposals @sandipanghos

@dhanashree-sawant
Copy link

dhanashree-sawant commented Dec 15, 2022

Proposal

Add nativeID to ScreenWrapper in src/pages/home/ReportScreen.js

<ScreenWrapper style={screenWrapperStyle} nativeID='report-screen-wrapper' >

Remove flex1 style from ScreenWrapper in src/pages/home/ReportScreen.js

const screenWrapperStyle = [styles.appContent, {marginTop: this.state.viewportOffsetTop}];

Add script to web/index.html before closing tag of body

<script>
        var isChromium = window.chrome;
        var winNav = window.navigator;
        var vendorName = winNav.vendor;
        var isIOSChrome = winNav.userAgent.match("CriOS");
        if (isIOSChrome || (isChromium !== null && typeof isChromium !== "undefined" && vendorName === "Google Inc.")) {
            if(window.innerHeight<800){
                if(document.getElementById('report-screen-wrapper')){
                    window.addEventListener("resize",function(){
                        document.getElementById('report-screen-wrapper').style.height=window.innerHeight;
                    });
                   
                }
            }
        }
    </script>

Explaination: The issue currently exist as one of the element inside ScreenWrapper is given width 100% and height 100% css. In order to keep the header static, height of that element will need to be changed dynamically whenever keyboard is loaded. I have attached a video by setting fixed height on how it will look after keyboard is opened. In the video currently the height is fixed but in the script it is dependent on viewport height.

Will ensure not to test chat again with Concierge.

Screenrecorder-2022-12-15-21-49-36-529.mp4

@mananjadhav
Copy link
Collaborator

Thanks for the proposal @dhanashree-sawant. I might agree with your explanation to an extent but not with the solution. I would recommend posting a solution that is more React way than adding a js script in the html file. Also note that this issue exists in mobile web, it works fine in other platforms I guess. We use React Native to code the app cross-platform across Web, Desktop (MacOs), Mobile Web, iOS and Android. Please refer to our contributing guides to submit a better proposal.

@dhanashree-sawant
Copy link

Hi @mananjadhav

Thanks for the response. I have worked on react based solution. Kindly review it once and let me know how it looks. The basic concept is same just wrote the solution in react.

src/pages/home/ReportScreen.js
@@ -215,7 +215,7 @@ class ReportScreen extends React.Component {
        const reportID = getReportID(this.props.route);
        const addWorkspaceRoomOrChatPendingAction = lodashGet(this.props.report, 'pendingFields.addWorkspaceRoom') || lodashGet(this.props.report, 'pendingFields.createChat');
        const addWorkspaceRoomOrChatErrors = lodashGet(this.props.report, 'errorFields.addWorkspaceRoom') || lodashGet(this.props.report, 'errorFields.createChat');
-       const screenWrapperStyle = [styles.appContent, styles.flex1, {marginTop: this.state.viewportOffsetTop}];
+       const screenWrapperStyle = [styles.appContent, this.props.isSmallScreenWidth? styles.appContentHeaderMobile : styles.flex1, {marginTop: this.state.viewportOffsetTop}];
src/styles/styles.js    
@@ -1203,26 +1203,29 @@
appContentHeader: {
        borderBottomWidth: 1,
        borderColor: themeColors.border,
        height: variables.contentHeaderHeight,
        justifyContent: 'center',
        display: 'flex',
        paddingRight: 20,
    },
+    appContentHeaderMobile:{
+      height:window.innerHeight,
+   },

@getusha
Copy link
Contributor

getusha commented Dec 16, 2022

@mananjadhav can you help me reproduce this? i am unable to, what i am missing here?

screen-recording-2022-12-16-at-91425-am_sOYaqqe8.mp4

@melvin-bot melvin-bot bot added the Overdue label Dec 19, 2022
@pecanoro
Copy link
Contributor

@getusha It is much easier to reproduce on those chats that have a lot of messages and so you can scroll up/back.

@melvin-bot melvin-bot bot removed the Overdue label Dec 19, 2022
@melvin-bot melvin-bot bot changed the title [$1000] mWeb - Chat header goes out of the view after opening keyboard [$2000] mWeb - Chat header goes out of the view after opening keyboard Dec 19, 2022
@pecanoro pecanoro changed the title [$2000] mWeb - Chat header goes out of the view after opening keyboard [HOLD] [$2000] mWeb - Chat header goes out of the view after opening keyboard Feb 13, 2023
@pecanoro
Copy link
Contributor

Maybe we should simply close this issue since I can't reproduce it anymore. But I will wait until the link issue gets fixed.

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels Feb 14, 2023
@mallenexpensify
Copy link
Contributor

Thanks for updating the title @pecanoro . This has been a weird/tough one. I bumped to weekly, let's leave open for a lil bit longer then close if we don't end up with a good reason to keep it open.

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Mar 10, 2023
@MelvinBot
Copy link

This issue has not been updated in over 15 days. @mananjadhav, @pecanoro, @mallenexpensify eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

@pecanoro
Copy link
Contributor

I forgot completely about this, but I tried to reproduce it again and there is no bug anymore (that I could see), so I am going to close

@thesahindia
Copy link
Member

Hey @pecanoro, can we reopen this? The reporting compensation is due.

@pecanoro pecanoro removed the Reviewing Has a PR in review label Mar 13, 2023
@mallenexpensify
Copy link
Contributor

mallenexpensify commented Mar 13, 2023

@thesahindia per CONTRIBUTING.md. a reporting bonus isn't due unless:

If the bug is fixed by a PR that is not associated with your bug report, then you will not be eligible for the corresponding compensation unless you can find the PR that fixed it and prove your proposal came first.

I'll reopen so this doesn't get lost if you comment.

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Monthly KSv2 labels Mar 13, 2023
@thesahindia
Copy link
Member

Hi @mallenexpensify! I think it was fixed by #14392.

@mallenexpensify
Copy link
Contributor

@thesahindia I just realized the CONTRIBUTING.md wording needed updating,

Also.... I just updated CONTRIBUTING.md to state

If the bug is fixed by a PR that is not associated with your bug report, then you will not be eligible for the corresponding compensation unless you can find the PR that fixed it and prove your proposal bug report came first.

Re: I think it was fixed by #14392.
You'll need to

find the PR that fixed it and prove your bug report came first.

@mananjadhav
Copy link
Collaborator

@mallenexpensify Can we discuss about the C+ payment for review of the proposals and the PR review (but that wasn't merged).

@thesahindia
Copy link
Member

prove your bug report came first

I think this should be applied only when the bug report is similar/same. I was thinking about the decision here. The current case is similar to that. This bug was different from #11463 which was fixed by #14392.

@mallenexpensify
Copy link
Contributor

Thanks @thesahindia , discussing internally.

@mananjadhav please follow the steps listed int he main C+ doc (which will be public before too long)

@mananjadhav
Copy link
Collaborator

@mallenexpensify I think it doesn't fall into the scenario that you've linked. It falls under this scenario where contributor has been hired for a job and we decide to close the job before it was successfully completed.

@mallenexpensify
Copy link
Contributor

ah.. it looks like you're right cuz we hired/assigned a contributor here. I did the lazy checked and looked to see if any contributor's were assigned. Sorry about that oversight

@thesahindia you're also do compensation because the bug reports weren't similar. Should we update the CONTRIBUTING.md to read

you can find the PR that fixed it and prove your proposal came first or that your bug report is not similar to the bug report associated with the PR that fixed the bug you reported.

@mananjadhav and @thesahindia can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~01386943f03f5f8c21

@mananjadhav
Copy link
Collaborator

@mallenexpensify The link didn't work, but I accepted the offer on Upwork.

@mallenexpensify
Copy link
Contributor

Interesting, the link is correct, I see This post is closed and is no longer accepting proposals. so that's likely the reason. It doesn't make sense though cuz you're hired for the job, why wouldn't YOU be able to see. I'll ask our contact at Upwork.

@mananjadhav
Copy link
Collaborator

@mallenexpensify I think because you made an offer and I accepted, it says Job is no longer available. I can see that in my list of contracts so I think it should be all good.

image

@mallenexpensify
Copy link
Contributor

Experiencing payment issues in Upwork, will pay @mananjadhav and @thesahindia soon

@mananjadhav
Copy link
Collaborator

@mallenexpensify Will you be able to get to this today?

@mallenexpensify
Copy link
Contributor

@mananjadhav and @thesahindia paid! Closing

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. Engineering External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
None yet
Development

No branches or pull requests