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

[$500] Feature Request: Add full screen expand button when editing a messge #15596

Closed
kavimuru opened this issue Mar 1, 2023 · 177 comments
Closed
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors NewFeature Something to build that is a new item.

Comments

@kavimuru
Copy link

kavimuru commented Mar 1, 2023

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


Problem:

When there's multiple lines being edited, there is a "full-screen expand" button in the upper-left of the compose window, but not when editing an existin

Solution:

Add the full-screen expand button to the upper-left when editing an existing comment

Context/Examples/Screenshots/Notes:

Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

full.screen.expand.button.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e878e8c8ed91741e
  • Upwork Job ID: 1632674482135707648
  • Last Price Increase: 2024-11-19
Issue OwnerCurrent Issue Owner: @s77rt
@kavimuru kavimuru added Weekly KSv2 NewFeature Something to build that is a new item. Bug Something is broken. Auto assigns a BugZero manager. labels Mar 1, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Mar 1, 2023
@MelvinBot
Copy link

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

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

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

@sonialiap
Copy link
Contributor

On web, when editing a message the edit composer opens to show the entire message if it's <=16 lines. The editor does not currently expand beyond 16 lines.

image

image

@sonialiap
Copy link
Contributor

I can't think of a reason for why we wouldn't want to allow expanding the editor to full screen 👍

@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot added the Overdue label Mar 6, 2023
@cristipaval
Copy link
Contributor

This could be external.

@melvin-bot melvin-bot bot removed the Overdue label Mar 6, 2023
@cristipaval cristipaval added the External Added to denote the issue can be worked on by a contributor label Mar 6, 2023
@melvin-bot melvin-bot bot unlocked this conversation Mar 6, 2023
@melvin-bot melvin-bot bot changed the title Feature Request: Add full screen expand button when editing a messge [$1000] Feature Request: Add full screen expand button when editing a messge Mar 6, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~01e878e8c8ed91741e

@MelvinBot
Copy link

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

@MelvinBot
Copy link

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

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

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

@tienifr

This comment was marked as outdated.

@s77rt
Copy link
Contributor

s77rt commented Mar 6, 2023

@tienifr I believe this still requires proposals to follow the template.

@tienifr
Copy link
Contributor

tienifr commented Mar 6, 2023

Edited by proposal-police: This proposal was edited at 2024-10-03 09:28:01 UTC.

@s77rt Sure, here's my proposal.

Proposal

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

When there's multiple lines being edited, there is a "full-screen expand" button in the upper-left of the compose window, but not when editing an existing.

What is the root cause of that problem?

This is because we don't have that feature yet when editing the message.

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

We can add this component

that is already in ReportActionComposer into the src/pages/home/report/ReportActionItemMessageEdit.js. Specifically above this component:

The logic should be similar as well:

  • There'll be a new setIsEditComposerFullSize to indicate which edit composer is currently full size (similar to the setIsComposerFullSize)
  • The full size state needs to be passed into the ReportActionItemMessageEdit by using useOnyx
    const [isComposerFullSize] = useOnyx(`reportActionsDrafts_ ${reportID}`, {
        selector: (actionDraft) => {
            return actionDraft?.[action.reportActionID].isFullSize;
        },
        initialValue: false
    });
  • We'll apply full size style to the composer using the full size state here
style={[styles.chatItemMessage, styles.flexRow, isComposerFullSize && {
                    height: windowHeight - HEADER_HEIGHT - (shouldShowComposeInput && mainComposerHeight) - 16 // padding
                }]}

We can store the main composer height in Onyx and get its value a.k.a mainComposerHeight in ReportActionItemMessageEdit

  • Enable isFullSize when users press Expand button
  • Disable isFullSize when users press Collapse button or dismiss edit mode
  • Hide main composer if isFullSize (from reportActionsDrafts_) is true and the edit composer is focused (from ONYXKEYS.SHOULD_SHOW_COMPOSE_INPUT)

What alternative solutions did you explore? (Optional)

NA

Result

This is a very draft screenshot of me implementing the change above, the exact design will depend on the design team.

web-resize.mp4

@s77rt
Copy link
Contributor

s77rt commented Mar 6, 2023

Thanks for the proposal @tienifr. One thing I'm concerned about is how will you make the composer take full height being a FlatList item.

@melvin-bot melvin-bot bot added the Overdue label Nov 11, 2024
@s77rt
Copy link
Contributor

s77rt commented Nov 11, 2024

Still no proposals yet

@melvin-bot melvin-bot bot removed the Overdue label Nov 11, 2024
Copy link

melvin-bot bot commented Nov 12, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@QichenZhu
Copy link
Contributor

@s77rt Could you summarize the reasons the previous proposals were not accepted, as this discussion is quite long?

Still waiting for proposals... (even for proposals that just make this work without Onyx)

What other key characteristics should new proposals have compared to the old ones?

Thanks!

@melvin-bot melvin-bot bot added the Overdue label Nov 14, 2024
@s77rt
Copy link
Contributor

s77rt commented Nov 14, 2024

@QichenZhu When clicking on the expand button the following are expected:

  • Message view takes full height
  • The focus and the cursor position are not lost
  • The unread indicator functions normally i.e. if someone sends you a message it should be marked as unread after you close the edit composer
  • Save state in onyx I don't think we should focus on this one as it's the reason we didn't move forward so far

@melvin-bot melvin-bot bot removed the Overdue label Nov 14, 2024
@s77rt
Copy link
Contributor

s77rt commented Nov 14, 2024

cc @tienifr I think your proposal would work too if we omit the save in onyx feature, would you be able to double check your proposal as well

Copy link

melvin-bot bot commented Nov 18, 2024

@s77rt, @sonialiap, @cristipaval Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Nov 18, 2024
@s77rt
Copy link
Contributor

s77rt commented Nov 18, 2024

Still looking for proposals

@melvin-bot melvin-bot bot removed the Overdue label Nov 18, 2024
@muttmuure muttmuure moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 19, 2024
Copy link

melvin-bot bot commented Nov 19, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Nov 22, 2024

@s77rt, @sonialiap, @cristipaval Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Nov 22, 2024
@s77rt
Copy link
Contributor

s77rt commented Nov 22, 2024

Same ^

@melvin-bot melvin-bot bot removed the Overdue label Nov 22, 2024
@sonialiap
Copy link
Contributor

@Expensify/design what do you guys think, should we update the behavior of how the comment editor window looks to add a full screen expand button? (OP is current)

Asking because this has been open since March so I wanted to double check whether this is something we want to update or if we're happy with the current functionality and I should close this issue.

@dannymcclain
Copy link
Contributor

I kinda don't see why we wouldn't. Curious for other design team thoughts but I feel like the "composer" should basically always be the same.

@shawnborton
Copy link
Contributor

I don't feel strongly really. I can see the argument for consistency but I can also see where this one might be complicated to pull off correctly since the message that is being edited might not necessarily be docked to the bottom of the view or anything.

@dannymcclain
Copy link
Contributor

I'd be fine doing nothing for now and coming back to it if it comes up as a problem for users.

@cristipaval
Copy link
Contributor

Thanks, @sonialiap and @Expensify/design! I'll close the issue for now and come back to it if the users want it, given the ton of other priorities we have atm.

@github-project-automation github-project-automation bot moved this from Bugs and Follow Up Issues to Done in [#whatsnext] #expense Nov 22, 2024
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. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors NewFeature Something to build that is a new item.
Projects
Status: Done
Development

No branches or pull requests