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

[Planning] Dragging the attachment modal to the bottom misplaces it - Reported by: @parasharrajat #5892

Closed
isagoico opened this issue Oct 15, 2021 · 39 comments
Assignees
Labels
Engineering Monthly KSv2 Planning Changes still in the thought process

Comments

@isagoico
Copy link

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. Navigate to a conversation
  2. Send a image
  3. Open the attachment preview
  4. Drag the modal to the bottom

Expected Result:

Attachment preview modal should not move

Actual Result:

Attachment preview modal moves after dragged and stays misplaced.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.1.8-0

Reproducible in staging?: Yes
Reproducible in production?: Yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation
image

Expensify/Expensify Issue URL:

Issue reported by: @parasharrajat
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1633858422069300?thread_ts=1633858370.069100&cid=C01GTK53T8Q

View all open jobs on GitHub

@MelvinBot
Copy link

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

@timszot timszot removed their assignment Oct 15, 2021
@timszot timszot added the External Added to denote the issue can be worked on by a contributor label Oct 15, 2021
@MelvinBot
Copy link

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

@mallenexpensify
Copy link
Contributor

mallenexpensify commented Oct 18, 2021

Is this intentional to allow the user to double check who they're sending the file/image to?
Asking about in thread https://expensify.slack.com/archives/C01GTK53T8Q/p1634566956408100?thread_ts=1633858370.069100&cid=C01GTK53T8Q

@mallenexpensify
Copy link
Contributor

Got some feedback from Bortman - link

I don’t think you should be able to drag the modal half way

@mallenexpensify
Copy link
Contributor

@MelvinBot MelvinBot added Weekly KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors and removed Daily KSv2 labels Oct 19, 2021
@MelvinBot
Copy link

Triggered auto assignment to @marcaaron (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@marcaaron
Copy link
Contributor

Taking hold off.

@mallenexpensify
Copy link
Contributor

Doubled price to $500

@MelvinBot MelvinBot removed the Overdue label Oct 29, 2021
@sidferreira
Copy link
Contributor

sidferreira commented Oct 29, 2021

withdrawn

@parasharrajat
Copy link
Member

@sidferreira I am sure we are not trying to remove the swiping behavior.

@sidferreira
Copy link
Contributor

@parasharrajat mind explaining what is the desired behaviour? Cause the slack discussion sounded otherwise...

@parasharrajat
Copy link
Member

parasharrajat commented Oct 29, 2021

Sure. This issue is specifically Affecting safari Mobile Web. IMO, as the issue says, when you drag it down it should be either closed or put back to normal. But not in the middle of the screen

Here is one discussion about it https://expensify.slack.com/archives/C01GTK53T8Q/p1634566956408100?thread_ts=1633858370.069100&cid=C01GTK53T8Q. Otherwise, CME(assigned engineer) will be able to help.

@sidferreira
Copy link
Contributor

@parasharrajat So, the issue happens in iOS < 15, but seems to work fine in iOS >= 15.

@sidferreira
Copy link
Contributor

@mallenexpensify It seems that adding overflow: hidden!important to the body tag fixes the issue.

So, my suggestion is to:

  • add .block-pull-to-refresh { overflow: hidden!important; } to the index.html file
  • add const blockPullToRefresh = navigator && navigator.appVersion && navigator.appVersion.indexOf('iPhone OS 1') >= 0; to AttachmentModal.js
  • add if (blockPullToRefresh) { document.querySelector('body').classList.remove('block-pull-to-refresh'); } to the onClose of the attachment modal
  • add if (blockPullToRefresh) { document.querySelector('body').classList.add('block-pull-to-refresh'); } to the onModalShowof the modal

This change would affect only iOS Web.

The change may be applied at BaseModal in order to ensure consistent behaviour.

@marcaaron
Copy link
Contributor

Sorry, I'm not sure what more to do to move this conversation forward - it needs to be brought up in the channel and get input from the wider team #expensify-open-source. I can put a planning label on it for now.

@marcaaron marcaaron added Planning Changes still in the thought process and removed Exported Help Wanted Apply this label when an issue is open to proposals by contributors External Added to denote the issue can be worked on by a contributor labels Nov 30, 2021
@marcaaron marcaaron removed their assignment Nov 30, 2021
@mallenexpensify
Copy link
Contributor

Gonna bump to monthly for now since it's not a high-value issue

@MelvinBot MelvinBot removed the Overdue label Dec 10, 2021
@mallenexpensify mallenexpensify added Monthly KSv2 and removed Weekly KSv2 labels Dec 10, 2021
@mallenexpensify mallenexpensify changed the title [$1000] Dragging the attachment modal to the bottom misplaces it - Reported by: @parasharrajat Dragging the attachment modal to the bottom misplaces it - Reported by: @parasharrajat Dec 29, 2021
@mallenexpensify mallenexpensify changed the title Dragging the attachment modal to the bottom misplaces it - Reported by: @parasharrajat [Planning] Dragging the attachment modal to the bottom misplaces it - Reported by: @parasharrajat Dec 29, 2021
@mallenexpensify
Copy link
Contributor

Interesting, I didn't think issues with the Planning label were supposed to go overdue, maybe that's currently only in the E/E repo and not E/App...

@mallenexpensify
Copy link
Contributor

No update...

@mallenexpensify
Copy link
Contributor

on hold/planning

@MelvinBot MelvinBot removed the Overdue label Mar 30, 2022
@melvin-bot melvin-bot bot added the Overdue label May 2, 2022
@melvin-bot

This comment was marked as off-topic.

@mallenexpensify
Copy link
Contributor

Still on hold...

@melvin-bot melvin-bot bot removed the Overdue label May 4, 2022
@mananjadhav
Copy link
Collaborator

mananjadhav commented May 4, 2022

Any specific reason we've put this into planning? I guess there isn't a solution to this with our RNModal implementation. Hence I recommended to disable it in the mWeb or we can close it?

cc - @mallenexpensify

@mallenexpensify
Copy link
Contributor

I'm not sure what's best, it seems like things kinda stalled. @marcaaron thoughts on keeping this on hold, closing or removing the planning label?

@marcaaron
Copy link
Contributor

I'm just gonna close it. Left a recommendation back then and doesn't seem like we came to any conclusion so - yeah seems even less valuable than we originally presumed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Monthly KSv2 Planning Changes still in the thought process
Projects
None yet
Development

No branches or pull requests

9 participants