Skip to content

Feature/5 chakra v1#10

Open
ghost wants to merge 5 commits into
productionfrom
feature/5-Chakra-v1
Open

Feature/5 chakra v1#10
ghost wants to merge 5 commits into
productionfrom
feature/5-Chakra-v1

Conversation

@ghost

@ghost ghost commented Oct 29, 2021

Copy link
Copy Markdown

closes #5

@ghost ghost added the enhancement New feature or request label Oct 29, 2021
@ghost
ghost requested review from joelwass and stripedpajamas October 29, 2021 22:42

@stripedpajamas stripedpajamas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks perf to me

@stripedpajamas

Copy link
Copy Markdown
Member

@korlando7 not sure why but the preview looks pretty bad https://deploy-preview-10--maintainer-ui.netlify.app/

Comment thread components/common/authProcess.js Outdated
Comment thread components/common/banner.js Outdated
Comment thread components/common/errorMessage.js Outdated
Comment thread components/common/stepper.js Outdated
Comment thread components/splash/cards/whyFBCards.js Outdated
Comment thread pages/_app.js
export default function Flossbank ({ Component, pageProps }) {
return (
<ThemeProvider theme={CustomTheme}>
<CSSReset />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you verify this is not needed anymore? and no color mode provider either?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Confirmed. These are both able to be removed with the V1 updates

Comment thread pages/brand-guidelines.js Outdated
Comment thread pages/dashboard.js Outdated
Comment thread pages/package/[packageId]/index.js Outdated
@joelwass

Copy link
Copy Markdown
Member

@korlando7 not sure why but the preview looks pretty bad https://deploy-preview-10--maintainer-ui.netlify.app/

Kev i think for the Icons they started naming their icons. so you'll have to import QuestionIcon from chakra/icons and then use that (etc etc for other icons)

for custom icons i believe you have to define a custom Icon then import it, look how enterprise portal did it

@ghost
ghost requested review from joelwass and stripedpajamas October 30, 2021 02:40
Comment thread components/common/banner.js Outdated
@@ -22,7 +22,7 @@ const Banner = ({ icon, children, onCloseClick }) => {
>
<Icon

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe this needs to be the CustomIcon because it seems chakra/icons doesn't have the celebration icon that we use for the banner

@joelwass joelwass left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think biggest issue i've found is icons.

log in and look at the account dropdown menu (should be full of icons)

and go across pages. you should never have a "?" icon, that means that chakra can't find it.

you can look at the enterprise portal to see how we managed migrating

Comment thread pages/brand-guidelines.js Outdated
<Link href='/images/flossbank_logo_assets.zip' download='flossbank_icons' padding='1rem'>
Download assets
<Icon marginLeft='1rem' name='download' size='1.75rem' />
<Icon marginLeft='1rem' name='download' w='1.75rem' h='1.75rem' />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same here, i believe this needs to be a CustomIcon since chakra doesn't have the "download" icon. or you can see if chakra icons has a similarly looking icon

@stripedpajamas stripedpajamas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ICONS

@ghost

ghost commented Nov 3, 2021

Copy link
Copy Markdown
Author

@stripedpajamas @joelwass fixed the icons. Let me know if you see any missing ones still.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update to chakra-ui v1

3 participants