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

TF-2143 Open composer into a new tab #2896

Open
wants to merge 10 commits into
base: refactor
Choose a base branch
from

Conversation

dab246
Copy link
Member

@dab246 dab246 commented May 29, 2024

Issue

#2143

Resolved

Screen.Recording.2024-07-09.at.13.51.38.online-video-cutter.com.mp4

Copy link

This PR has been deployed to https://linagora.github.io/tmail-flutter/2896.

@chibenwa
Copy link
Member

chibenwa commented May 29, 2024

Looks cool!

Few points:

1/

GIVEN I open composer in TAB A and start input
WHEN chick the openCOmposer in TAB A
THEN the composer is opened into TAB B
AND the composer into TAB A is closed

is this the case?

2/ Suggestion: save a draft before opening into a new tab.

Not that costly I suppose and that way we prevent input loss.

@dab246
Copy link
Member Author

dab246 commented May 29, 2024

Looks cool!

Few points:

1/

GIVEN I open composer in TAB A and start input WHEN chick the openCOmposer in TAB A THEN the composer is opened into TAB B AND the composer into TAB A is closed

is this the case?

  • Currently we do not close composer on Tab A when we do Open new tab on Tab B.
  • I don't think we should close composer in Tab A when opening Tab B. It's the same as opening a new tab of an email that we implemented before.
  • ANW, if you think it makes sense in terms of UX then we can implement it.

2/ Suggestion: save a draft before opening into a new tab.

Not that costly I suppose and that way we prevent input loss.

I must confirm that the save a draft suggestion gives us the ability to avoid losing data when opening a new tab. But we have to trade off in terms of speed and processing time for this task.

  • When performing save a draft, we are forced to call a request to the server. Depending on the server's response time, we will have the results to perform the next steps.
  • If a problem occurs during the save a draft process (network error or any server error), we will not be able to continue with the next steps.

@chibenwa
Copy link
Member

Then Ok to me with current proposal.
Let's see user feedback then.

@tddang-linagora
Copy link
Contributor

Suggestion:

  • Test with rich text

@tddang-linagora
Copy link
Contributor

Suggestion:

  • Test input an email to From/CC/BCC, but not choose the suggested pop up yet, and tap open in new tab button

@dab246
Copy link
Member Author

dab246 commented May 31, 2024

Suggestion:

  • Test with rich text

Work well.

Screen.Recording.2024-05-31.at.10.45.03.mov

@dab246
Copy link
Member Author

dab246 commented May 31, 2024

Suggestion:

  • Test input an email to From/CC/BCC, but not choose the suggested pop up yet, and tap open in new tab button

IMO, we should not keep the same text on TextField for To/Cc/Bcc inputs but we should allow automatic tag generation for To/Cc/Bcc inputs when clicking open new tab. It will make storage simpler, still ensuring user data is not lost, although the UX aspect is not very good.

Screen.Recording.2024-05-31.at.11.01.57.mov

Copy link
Contributor

@tddang-linagora tddang-linagora left a comment

Choose a reason for hiding this comment

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

LGTM

@hoangdat
Copy link
Member

hoangdat commented Jun 5, 2024

Build test

  • Android
  • Web

@tddang-linagora
Copy link
Contributor

tddang-linagora commented Jun 5, 2024

Open composer in new tab should not exist on mobile

No-composer-open-in-new-tab-ios.mov

@hoangdat
Copy link
Member

hoangdat commented Jun 5, 2024

@dab246 please don't forget to change destination to master

@dab246 dab246 changed the base branch from v0.11.3-patch4-dev to master June 5, 2024 08:44
@dab246 dab246 changed the base branch from master to v0.11.3-patch4-dev June 5, 2024 10:00
@dab246 dab246 dismissed tddang-linagora’s stale review June 5, 2024 10:00

The base branch was changed.

@dab246
Copy link
Member Author

dab246 commented Jun 5, 2024

@dab246 please don't forget to change destination to master

Sorry, my mistake. It depends heavily on the dev branch because it takes the results of the process of changing the send email in composer. So let's wait for the dev branch to merge into master (#2911) and then cherry pick it.

@dab246 dab246 force-pushed the improvement/tf-2143-open-composer-into-a-new-tab branch from 1cc848f to 0140832 Compare June 13, 2024 07:04
@dab246 dab246 changed the base branch from v0.11.3-patch4-dev to refactor June 13, 2024 07:07
@dab246
Copy link
Member Author

dab246 commented Jun 13, 2024

Rebase to the refactor branch is complete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants