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

fix: profile domain tweaks #2183

Closed
wants to merge 4 commits into from
Closed

Conversation

brenopolanski
Copy link
Contributor

Summary

  • Use .then() to capture promise results
  • Add i18n
  • Fix Msq icon
  • Update tests

Checklist

  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@ghost ghost added Complexity: Low Less than 64 lines changed. Type: Bugfix The pull request fixes an incorrect functionality or behaviour. labels Jun 18, 2020
@brenopolanski brenopolanski marked this pull request as draft June 18, 2020 13:23
setProfiles(env.profiles().all());
env.profiles()
.all()
.then((profiles: any) => setProfiles(profiles));
Copy link
Contributor

@faustbrian faustbrian Jun 18, 2020

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@brenopolanski brenopolanski Jun 18, 2020

Choose a reason for hiding this comment

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

Yeah, you are right! I was just looking at the story https://github.com/ArkEcosystem/desktop-wallet/blob/3.0-react/src/domains/profile/pages/Welcome/Welcome.stories.tsx#L18-L30. So, for this case the promise will always log pending as long as its results are not resolved yet.

key={profile.id()}
name={profile.name}
avatar={profile.avatar}
balance={profile.balance}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't exist on the profile yet.

balance="0"
key={profile.id()}
name={profile.name}
avatar={profile.avatar}
Copy link
Contributor

Choose a reason for hiding this comment

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

avatar is a method.

avatar={profile.avatar()}
balance="0"
key={profile.id()}
name={profile.name}
Copy link
Contributor

Choose a reason for hiding this comment

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

name is a method.

name={profile.name}
avatar={profile.avatar}
balance={profile.balance}
key={profile.id}
Copy link
Contributor

Choose a reason for hiding this comment

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

id is a method.

export const Default = () => {
const env = {
profiles: () => ({
all: async () => new Promise((resolve) => resolve([])),
Copy link
Contributor

Choose a reason for hiding this comment

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

<div className="block w-12 h-12 mx-auto rounded-full sm:mx-0 sm:flex-shrink-0">
<img
className="rounded-full"
src={avatar}
Copy link
Contributor

Choose a reason for hiding this comment

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

The avatar is purely CSS, not an image.

Copy link
Contributor

@faustbrian faustbrian left a comment

Choose a reason for hiding this comment

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

Pretty much all changes in this PR revert things that were done on purpose because of how the SDK works and certain is provided. What's the reasoning for that?

@brenopolanski brenopolanski deleted the fix/profile-domain-tweaks branch June 18, 2020 14:49
@brenopolanski
Copy link
Contributor Author

Pretty much all changes in this PR revert things that were done on purpose because of how the SDK works and certain is provided. What's the reasoning for that?

My mistake sorry. PR closed!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Complexity: Low Less than 64 lines changed. Type: Bugfix The pull request fixes an incorrect functionality or behaviour.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants