Skip to content

Conversation

@mahmud-bn
Copy link
Contributor

@mahmud-bn mahmud-bn commented May 16, 2022

Description

Checklist

  • The version field in package.json is incremented following semantic versioning
  • The box that allows repo maintainers to update this PR is checked
  • I tested locally to make sure this feature/fix works
  • This PR passes the Circle CI checks

@mahmud-bn mahmud-bn requested a review from Adamj1232 May 16, 2022 20:17
@gesquinca gesquinca self-requested a review May 16, 2022 20:36
@gesquinca
Copy link
Contributor

How would a dev configure which component they would like to use for the minimized state of account center?
And how would they configure different ones for desktop and mobile?

@mahmud-bn
Copy link
Contributor Author

mahmud-bn commented May 16, 2022

@gesquinca
Still waiting for Murat's feedback on this but there will be an account center initialization addition for devs to make appropriate configurations.

Right now, the current opinion is: Desktop should default to minimal: false, and mobile should default to minimal: true

@gesquinca
Copy link
Contributor

is there a way to test this on the demo?

Copy link
Contributor

@lnbc1QWFyb24 lnbc1QWFyb24 left a comment

Choose a reason for hiding this comment

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

Added some requested changes from looking at the code.

Also I can't see in the diff changes to the Onboard initialization code that allow for accountCenter mobile configuration and the extra field in the desktop configuration and the associated validation?

@mahmud-bn
Copy link
Contributor Author

mahmud-bn commented May 17, 2022

Also I can't see in the diff changes to the Onboard initialization code that allow for accountCenter mobile configuration and the extra field in the desktop configuration and the associated validation?
@aaronbarnardsound That's for the next card that allows added configuration

@mahmud-bn mahmud-bn closed this May 17, 2022
@mahmud-bn mahmud-bn reopened this May 17, 2022
Copy link
Contributor

@Adamj1232 Adamj1232 left a comment

Choose a reason for hiding this comment

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

Looks good! Good work sticking it out!
I would wait for @aaronbarnardsound for a final approval but code looks clean and works well from what I can tell

@gesquinca
Copy link
Contributor

I'm seeing the Micro.svelte component rendering with a size of 84 x 56 px. This designs specify 80 x 56 px.

I'm also still seeing Mimimized.svelte and Maximized.svelte components rendering with different widths. This should not be the case. They should have the same width.

Copy link
Contributor

@lnbc1QWFyb24 lnbc1QWFyb24 left a comment

Choose a reason for hiding this comment

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

A few small requested changes.

@mahmud-bn mahmud-bn requested a review from lnbc1QWFyb24 May 24, 2022 11:51
Copy link
Contributor

@lnbc1QWFyb24 lnbc1QWFyb24 left a comment

Choose a reason for hiding this comment

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

Looks good to me! 🎉

@mahmud-bn mahmud-bn merged commit 1d9151c into v2-web3-onboard-develop May 25, 2022
@mahmud-bn mahmud-bn deleted the feature/minimal-account-center branch May 25, 2022 02:06
tokensex pushed a commit to tokensex/web3-onboard that referenced this pull request May 30, 2022
… feature (thirdweb-dev#1004)

* Minimized the account center button removing balance, address, and network switch

* Remove disconnect all wallets and connect another wallet option on mobile

* Remove disconnect all wallets and connect another wallet option on mobile

* Version bump

* Formatting

* Version bump

* Added yarn lock to gitignore

* Version bump

* Yarn lock fix

* Delete yarn.lock

* Spacing and documentation update

* Documentation update

* Demo core version package update

* fixes

* Desktop and Mobile configurations

* Refine constant configurations

* Refine constant configurations

* Remove commented code

* Remove empty lines

* Packages bump

* Documentation update

* Cleanup

* Conflicts fix

* styling fixes

* styling fixes

* fixes

* Change width based on micro state

* Remove yarn.lock from demo

* Revert yarn.lock changes

Co-authored-by: Aaron Barnard <[email protected]>
@funiimunii
Copy link

The current validation doesnt allow for setting minimal

const initOptions = Joi.object({
    wallets: walletInit,
    chains: chains.required(),
    appMetadata: appMetadata,
    i18n: Joi.object().unknown(),
    accountCenter: Joi.object({
        desktop: Joi.object({
            enabled: Joi.boolean(),
            position: accountCenterPosition
        }),
        mobile: Joi.object({
            enabled: Joi.boolean(),
            position: accountCenterPosition
        })
    })
});

@mahmud-bn
Copy link
Contributor Author

The current validation doesn't allow for setting minimal

Hi @funiimunii,
We do have the current validation for minimal. You can look at the changes tab. We've also had it merged in the main branch:

const initOptions = Joi.object({
  wallets: walletInit,
  chains: chains.required(),
  appMetadata: appMetadata,
  i18n: Joi.object().unknown(),
  accountCenter: Joi.object({
    desktop: Joi.object({
      enabled: Joi.boolean(),
      minimal: Joi.boolean(),
      position: accountCenterPosition
    }),
    mobile: Joi.object({
      enabled: Joi.boolean(),
      minimal: Joi.boolean(),
      position: accountCenterPosition,
    })
  })
}) 

@funiimunii
Copy link

aghh okay. we are using @web3-onboard/react npm package 2.1.6. Will upgrade to 2.1.7 and hopefully these made the cut?

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.

6 participants