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

Onboarding Chunk 2 #3083

Merged
merged 231 commits into from
Jul 26, 2024
Merged

Onboarding Chunk 2 #3083

merged 231 commits into from
Jul 26, 2024

Conversation

yougotwill
Copy link
Collaborator

@yougotwill yougotwill commented Apr 24, 2024

Code review has been done on Will’s fork:
yougotwill#43

Edit: Chunk 1 has been merged so we can do the final review here

moved some parts into the leftpanesectionheader
this will replace the sessionideditable eventually still needs error handling etc
only show button when users enters something
created new ActionRow component
since we swapped some toast messages for input messages
updated error correction to 25%
updated account id and recovery password qr codes with logos
qr code logos can now be svgs for better rendering performance
some config messages are not handled correctly and we need to know why
disabled conversation header functionality
Copy link
Collaborator Author

@yougotwill yougotwill left a comment

Choose a reason for hiding this comment

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

Responded

ts/components/DebugLogView.tsx Show resolved Hide resolved
ts/components/lightbox/LightboxGallery.tsx Outdated Show resolved Hide resolved
ts/components/lightbox/LightboxGallery.tsx Outdated Show resolved Hide resolved
ts/components/registration/components/BackButton.tsx Outdated Show resolved Hide resolved
ts/state/ducks/theme.tsx Show resolved Hide resolved
ts/util/qrCodes.tsx Outdated Show resolved Hide resolved
ts/util/accountManager.ts Outdated Show resolved Hide resolved
ts/session/constants.ts Outdated Show resolved Hide resolved
ts/util/accountManager.ts Outdated Show resolved Hide resolved
@yougotwill yougotwill force-pushed the feat/ses-825/onboarding2 branch from 6ce969a to 8ea7daa Compare July 24, 2024 09:07
ts/components/lightbox/LightboxGallery.tsx Outdated Show resolved Hide resolved
ts/components/registration/stages/CreateAccount.tsx Outdated Show resolved Hide resolved
ts/data/opengroups.ts Outdated Show resolved Hide resolved
const result = renderComponent(
// NOTE we need to test the pubkey to color generation and ordering with appium. Since we can't access the value of a css variable in with the current unit test setup
<AvatarPlaceHolder
diameter={AvatarSize.XL}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we not at least verify that the string var(--color-xxx) is the right one for each pubkeys?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unfortunately not because it possible from the component because the color is just the primary color variable. I wanted avoid abstrating the color generation into a function. Which is what strated unit testing components in the first place. I can do that and then we can test the individual color values.

i.e. The color prop returned is var(--primar-color) it does not resolve to a hex code

ts/util/accountManager.ts Outdated Show resolved Hide resolved
@Bilb Bilb merged commit 1f7e99f into oxen-io:unstable Jul 26, 2024
6 checks passed
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.

2 participants