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

Improved Profile Picture UI #2765

Conversation

yougotwill
Copy link
Collaborator

@yougotwill yougotwill commented May 23, 2023

  • Converted EditProfileDialog to a functional component.
  • Added a modal to edit the user's profile picture
    • Supports updating and removing the profile picture
  • Added proper typings to icons and added thumbnail icon
  • Updated profile picture integration tests
  • Sort localised keys alphabetically

@yougotwill yougotwill changed the title WIP: Improved Profile Picture UI Improved Profile Picture UI May 23, 2023
ts/components/dialog/ModalContainer.tsx Outdated Show resolved Hide resolved
ts/components/icon/Icons.tsx Outdated Show resolved Hide resolved
ts/types/LocalizerKeys.ts Outdated Show resolved Hide resolved
ts/components/dialog/EditProfileDialog.tsx Show resolved Hide resolved
ts/receiver/userProfileImageUpdates.ts Outdated Show resolved Hide resolved
@burtonemily burtonemily self-assigned this May 26, 2023
@yougotwill yougotwill changed the base branch from clearnet to unstable July 10, 2023 06:38
@yougotwill yougotwill changed the base branch from unstable to clearnet July 10, 2023 06:39
@yougotwill yougotwill force-pushed the feature/ses-476/remove-profile-picture branch 2 times, most recently from 4705c10 to a955ad2 Compare July 10, 2023 07:40
@yougotwill yougotwill changed the base branch from clearnet to unstable July 10, 2023 07:40
@yougotwill yougotwill closed this Jul 10, 2023
@yougotwill yougotwill deleted the feature/ses-476/remove-profile-picture branch July 10, 2023 23:08
@yougotwill yougotwill restored the feature/ses-476/remove-profile-picture branch July 11, 2023 05:33
@yougotwill yougotwill reopened this Jul 11, 2023
@yougotwill yougotwill force-pushed the feature/ses-476/remove-profile-picture branch from 853011a to 41d2760 Compare July 18, 2023 03:49
yougotwill and others added 4 commits July 18, 2023 17:05
except for linked device profile sync, need to update avatar-update-blue files for linux
- loop until match when validating screenshot
- wait 15s and take screenshot when updating screenshot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a specific reason to change the test name here? If there is I will update the mobile platforms but otherwise we should keep the tests name unchanged.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The reason is so that we can update all profile picture snapshots through one command.

If we do yarn run integration-test-snapshots it does npx playwright test -g 'profile picture' --update-snapshots" which will run the "Check profile picture syncs" test and create a new snapshot. The "Change avatar" test had to be run manually. By making it profile picture both tests run off the same command.

So it's more of a convenience thing. Happy to change it back if it is going to cause problems?

Copy link
Collaborator

Choose a reason for hiding this comment

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

So its updating both the linked device test and the change avatar test in one go, if you change the name of the change avatar test? Is it a script that you've created or is this playwright functionality?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's using the "-g" flag it will look for any tests that contain the string 'profile picture' and only those 2 tests match. No additional scripts have been added.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, will update the appium tests to match👌

@Bilb
Copy link
Collaborator

Bilb commented Aug 21, 2023

Closing this inf favor if #2869

@Bilb Bilb closed this Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants