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

feat(chats): Allow image copy #677

Merged
merged 13 commits into from
Oct 16, 2024
Merged

feat(chats): Allow image copy #677

merged 13 commits into from
Oct 16, 2024

Conversation

Flemmli97
Copy link
Contributor

What this PR does 📖

  • Allows copying image in chats when right clicking on it

Which issue(s) this PR fixes 🔨

Special notes for reviewers 🗒️

Browsers (or at least safari) dont seem to allow writing to the clipboard and there is no permission request for that afaik. Because of this copying anything other than text seems to not be possible.

@github-actions github-actions bot added the Missing dev review Two dev reviews are required on PR label Oct 4, 2024
Copy link

github-actions bot commented Oct 4, 2024

Download the app installers for this pull request:

Copy link

github-actions bot commented Oct 4, 2024

Automated Test Results

passed  110 passed
skipped  15 skipped

Details

stats  125 tests across 16 suites
duration  13 minutes, 30 seconds
commit  e9bdbe6

Skipped tests

Automated Tests on Chrome Desktop › 03-friends-two-instances.spec.ts › Two instances tests - Friends and Chats › B8 to B14 - Quick Profile Local and Remote - Updating note
Automated Tests on Chrome Desktop › 03-friends-two-instances.spec.ts › Two instances tests - Friends and Chats › B8 to B14 - Quick Profile Local - Updating username, status, banner and profile picture
Automated Tests on Chrome Desktop › 03-friends-two-instances.spec.ts › Two instances tests - Friends and Chats › B8 to B14 - Quick Profile Remote - Updating username, status, banner and profile picture
Automated Tests on Chrome Desktop › 09-settings-customizations.spec.ts › Settings Customization Tests › K2 - Font dropdown should show expected font names
Automated Tests on Chrome Desktop › 09-settings-customizations.spec.ts › Settings Customization Tests › K15 - Emoji Font dropdown should show expected emoji font names
Automated Tests on Chrome Desktop › 09-settings-customizations.spec.ts › Settings Customization Tests › K18 - Default Profile Picture Style dropdown should show expected default profile picture styles
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O1, 06 - Message at top of page and custom keybinds listed correctly
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O2, 04 - Clicking a key should activate the Recorded Keys - User can save keybind
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O3 - Action dropdown should display correct keybind actions
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O5 - Clicking Cancel should cancel any custom keybinding the user was trying to add
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O7 - Highlighted border should display when user clicks cancel
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O8 - Clicking Revert Keybindings should revert any custom keybindings the user has saved
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O9 - Highlighted border should be displayed when clicking Revert Keybindings
Automated Tests on Chrome Desktop › 13-settings-keybinds.spec.ts › Settings Keybinds Tests › O10 - Clicking the backwards arrow should revert specific custom keybinding
Automated Tests on Chrome Desktop › 20-settings-developer.spec.ts › Settings Developer Tests › T4 - Clicking Clear State should clear users state

Copy link

github-actions bot commented Oct 4, 2024

Automated tests execution is complete! You can find the Playwright test report here and the Allure Test Report here

@github-actions github-actions bot added the Missing fixing conflict Needs to fix conflicts label Oct 4, 2024
@phillsatellite
Copy link
Contributor

I've done some testing with Sara and were having a few issues

We cant get copy image to work in general. Whenever I click copy an error appears in the logs and then when we try to paste nothing pastes

Sara was able to get a notification that asked her for permission to be able to copy images but I never received that notification. This was tested on Chrome and Brave

Screenshot 2024-10-04 at 6 14 15 PM
Screen_Recording_2024-10-04_at_6.12.43_PM.mov

@phillsatellite phillsatellite added the QA Requested Changes Changes need to be addressed, something is not working as expected. label Oct 4, 2024
@Flemmli97
Copy link
Contributor Author

seems it only supports png writes to clipboard. so i changed it to always write as png

@Flemmli97 Flemmli97 removed QA Requested Changes Changes need to be addressed, something is not working as expected. Missing fixing conflict Needs to fix conflicts labels Oct 7, 2024
@phillsatellite
Copy link
Contributor

Ive tried retesting with the latest and still having some issues

I tried png, jpg, jpeg, webp and still seems to not work. I noticed the error changed now as well I posted the new error below

Screenshot 2024-10-07 at 12 57 30 PM
Screen.Recording.2024-10-07.at.12.57.03.PM.mov

@phillsatellite phillsatellite added the QA Requested Changes Changes need to be addressed, something is not working as expected. label Oct 7, 2024
@stavares843
Copy link
Member

same here

@Flemmli97 Flemmli97 removed the QA Requested Changes Changes need to be addressed, something is not working as expected. label Oct 9, 2024
Copy link
Collaborator

@lgmarchi lgmarchi left a comment

Choose a reason for hiding this comment

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

Make a relevant question about image types.

src/routes/chat/+page.svelte Show resolved Hide resolved
@lgmarchi lgmarchi added the QA Requested Changes Changes need to be addressed, something is not working as expected. label Oct 9, 2024
@Flemmli97 Flemmli97 removed the QA Requested Changes Changes need to be addressed, something is not working as expected. label Oct 11, 2024
@phillsatellite
Copy link
Contributor

phillsatellite commented Oct 11, 2024

Whatever I copy I cant paste it in Uplink but otherwise it does seem to be working. Will approve

Screen.Recording.2024-10-11.at.12.15.02.PM.mov

@phillsatellite phillsatellite added the QA Approved PR has been tested by QA Team label Oct 11, 2024
@Flemmli97
Copy link
Contributor Author

Whatever I copy I cant paste it in Uplink but otherwise it does seem to be working. Will approve

Screen.Recording.2024-10-11.at.12.15.02.PM.mov

i think we dont have that feature for chatbar yet. but i tried pasting it elsewhere which worked

@github-actions github-actions bot added the Missing fixing conflict Needs to fix conflicts label Oct 11, 2024
@Flemmli97 Flemmli97 removed the Missing fixing conflict Needs to fix conflicts label Oct 15, 2024
@phillsatellite phillsatellite removed the Missing dev review Two dev reviews are required on PR label Oct 16, 2024
@phillsatellite phillsatellite merged commit 39be3e1 into dev Oct 16, 2024
10 checks passed
@phillsatellite phillsatellite deleted the file_copy branch October 16, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Approved PR has been tested by QA Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

improvement(chats): context menu for image messages should give option to copy image and copy text
6 participants