Skip to content
This repository has been archived by the owner on Dec 15, 2023. It is now read-only.

fix(avatar): change avatar seed algorithm to sync with android #1182

Merged
merged 1 commit into from
Sep 18, 2023

Conversation

braddialpad
Copy link
Contributor

fix(avatar): change avatar seed algorithm to sync with android

🛠️ Type Of Change

  • Fix
  • Feature
  • Refactoring
  • Documentation

📖 Description

Meant to do this a while back but forgot. Updated our avatar random generation algorithm to sync with android.

💡 Context

Our previous hashing algo FNV was not easy to implement on android, so I've updated our code to use the same algo as java's "hashCode". Our user avatar colors on web and mobile will be in sync after this change.

The exact same tests I've implemented here are also implemented on android, so we should be able to remain in sync.

📝 Checklist

  • I have reviewed my changes
  • I have added tests
  • I have added all relevant documentation
  • I have validated components with a screen reader
  • I have validated components keyboard navigation
  • I have considered the performance impact of my change
  • I have checked that my change did not significantly increase bundle size
  • I am exporting any new components or constants in the index.js in the component directory
  • I am exporting any new components or constants in the index.js in the root

🔮 Next Steps

Needs to be updated in Dialpad

🔗 Sources

https://www.w3schools.com/java/ref_string_hashcode.asp

@github-actions
Copy link

Thanks for contributing to Dialtone Vue! Please read below for some important info regarding Vue 3 compatibility.

Currently we need to maintain two branches in Dialtone Vue, one for Vue 2, one for Vue 3.

This means you must create two PRs for every feature change you make. One into staging and one into staging-vue3.

Many times the change you have made in Vue 2 will be identical to the change you need to make in Vue 3. To make this easier we have made a script that can copy your changes from this branch to a new branch off of staging-vue3 suffixed with -vue3.

run ./copy_pr_vue3.sh from the root dialtone-vue directory.

Once the new branch is created, you will need to look at your code to make sure it still makes sense and test that your changes all work in vue 3. If everything is good you can push it and create a PR into staging-vue3.

It is a required check for every PR to have a corresponding branch called yourbranch-vue3 so that we do not forget to do it. In the special case that you need to make a change to vue2 without making it in vue3, you can set the label vue2-only on this PR.

I got "commit SHA is a merge but no -m option was given."

This happens if there are merge commits in your branch. It's no problem, you can simply skip them with git cherry-pick --skip. We don't want to copy merge commits to the Vue 3 branch.

What if I make more changes to my vue 2 branch after running ./copy_pr_vue3.sh?

You can copy these to the existing -vue3 branch by running the script with a git SHA param like so:

./copy_pr_vue3.sh 2a78db7

where 2a78db7 is the last commit from your branch that was copied to the other branch (all commits after this will be copied)

If it's just one or two commits, you may prefer to just manually use git cherry-pick which will work fine as well.

What if I get a conflict?

It's possible to get a conflict when running copy_pr_vue3.sh as there are differences in Vue 2 and Vue 3 code. If this happens you can manually fix the conflict, commit it and do git cherry-pick --continue.

@github-actions
Copy link

✔️ Deploy Preview ready!
😎 Browse the preview: https://vue.dialpad.design/deploy-previews/pr-1182/
🔨 If you experience an SSL issue then wait 2 minutes and try again.

const hash = fnv.hash(seed);
return array[hash.value % array.length];
const hash = javaHashCode(seed);
return array[Math.abs(hash) % array.length];
} else {

Choose a reason for hiding this comment

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

Is this function only used for Avatar or are there any other components that make use of it? If not, maybe we can rename it to getAvatarBackgroundColor or something more specific?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was it could be used for any array I don't think it needs to be specific to avatar.

Choose a reason for hiding this comment

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

Awesome, having one algo for deciding coloring would be great!

Copy link
Contributor

@iropolo iropolo left a comment

Choose a reason for hiding this comment

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

Thanks

@github-actions github-actions bot added the visual-test-ready Add this tag when the PR is ready for visual test, to trigger GHA visual tests label Sep 17, 2023
@braddialpad braddialpad merged commit 53a09f3 into staging Sep 18, 2023
18 checks passed
@braddialpad braddialpad deleted the fix/avatar-seed-algo branch September 18, 2023 16:09
juliodialpad pushed a commit that referenced this pull request Sep 20, 2023
# [2.97.0](v2.96.0...v2.97.0) (2023-09-20)

### Bug Fixes

* add virtual "prompt" for visual tests on PR ([#1192](#1192)) ([4a41d02](4a41d02))
* **Avatar:** change avatar seed algorithm to sync with android ([#1182](#1182)) ([53a09f3](53a09f3))
* **Contact Info, Feed Item Row, Item Layout:** alignment issues ([#1186](#1186)) ([bdaa355](bdaa355))
* feed pill not exported ([#1180](#1180)) ([792b838](792b838))
* **Item Layout:** selected slot alignment ([#1194](#1194)) ([09c37c4](09c37c4))
* **Recipe Callbar Button With Popover:** change event name and sync open state ([#1183](#1183)) ([b91dc02](b91dc02))

### Features

* **Avatar:** add clickable behavior ([#1176](#1176)) ([2b80018](2b80018))
* **Callbox:** add clickable behavior ([#1196](#1196)) ([bf5ac2b](bf5ac2b))
@github-actions
Copy link

🎉 This PR is included in version 2.97.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
released visual-test-ready Add this tag when the PR is ready for visual test, to trigger GHA visual tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants