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

Introduce html-based AboutGuide. Normalize dialogs. Use theme. #531

Merged
merged 32 commits into from Feb 2, 2023

Conversation

pablo-mayrgundter
Copy link
Member

@pablo-mayrgundter pablo-mayrgundter commented Dec 23, 2022

Use some Mui dialog components, more typography, theme support, revive dialog header icons. Move About into subfolder.

…components, typography, theme support, revive dialog header icons. Move About into subfolder.
@netlify
Copy link

netlify bot commented Dec 23, 2022

Deploy Preview for bldrs-share ready!

Name Link
🔨 Latest commit b9579e6
🔍 Latest deploy log https://app.netlify.com/sites/bldrs-share/deploys/63dbb7730106650008a3e84b
😎 Deploy Preview https://deploy-preview-531--bldrs-share.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@pablo-mayrgundter pablo-mayrgundter self-assigned this Dec 23, 2022
@pablo-mayrgundter pablo-mayrgundter added the code Code cleanup, refactoring, style and lint label Dec 23, 2022
@OlegMoshkovich
Copy link
Member

I don't know what state of work this is, but on the first glance a lot of elements need to be edited.

@OlegMoshkovich
Copy link
Member

image

the dialogs don't look good with the headers.

@OlegMoshkovich
Copy link
Member

OlegMoshkovich commented Dec 23, 2022

why is this underline and bold?
image

@OlegMoshkovich
Copy link
Member

image

this is a wrong icon.

@OlegMoshkovich
Copy link
Member

OlegMoshkovich commented Dec 23, 2022

image

b is enormous and the logo image in the middle is broken.

Fixed broken background img

@OlegMoshkovich
Copy link
Member

image

share and about icons are smaller.

@pablo-mayrgundter
Copy link
Member Author

image

this is a wrong icon.

Yep, fixed.

@OlegMoshkovich
Copy link
Member

OlegMoshkovich commented Dec 23, 2022

Take a look at the design dialogs
here

Maybe dialog headers can be styled differently, without the icons
image

image

@pablo-mayrgundter
Copy link
Member Author

image

the dialogs don't look good with the headers.

Agreed.. I like the icon though. See what you think about moving them to left instead of above.

@pablo-mayrgundter
Copy link
Member Author

pablo-mayrgundter commented Dec 24, 2022

image

share and about icons are smaller.

Fixed. There's now 1 place where svg sizes are set, in Share.jsx#GlobalStyles, and small X icons are className="closeButton"

@OlegMoshkovich
Copy link
Member

OlegMoshkovich commented Dec 29, 2022

@pablo-mayrgundter can the operations group go back to this format?
image

image

the divider is too wide and the icons are too spaced out

The icons should be larger.

@OlegMoshkovich
Copy link
Member

image

B disappeared from the left corner.

@OlegMoshkovich
Copy link
Member

OlegMoshkovich commented Dec 29, 2022

image

this is still not balanced. B is too big, I think, what about you?
mixed feelings about bldrs.ai underneath.

image

BLDRS icon in the guide graphic, it should be vertically centered.

also for this type of work, you mentioned you would like to use the storybook,
what about creating a storybook component for the about dialog and just a general dialog?

@OlegMoshkovich
Copy link
Member

OlegMoshkovich commented Dec 29, 2022

image

there is too much space between the button and the dialog content. the vertical scroll bar should not be displayed. the placement of the X is weird, maybe should be higher.

@OlegMoshkovich
Copy link
Member

image

all of the scroll bars are green :) did you connect it to the theme?

@OlegMoshkovich OlegMoshkovich marked this pull request as ready for review January 8, 2023 08:41
@OlegMoshkovich OlegMoshkovich marked this pull request as draft January 10, 2023 13:36
@OlegMoshkovich
Copy link
Member

@pablo-mayrgundter dialog close buttons do not work on mobile.

@pablo-mayrgundter
Copy link
Member Author

PTAL

@pablo-mayrgundter pablo-mayrgundter marked this pull request as ready for review January 27, 2023 15:57
@pablo-mayrgundter pablo-mayrgundter changed the title Introduce html-based AboutGuide. Normalize dialogs. Refactoring. Introduce html-based AboutGuide. Normalize dialogs. Use theme. Jan 31, 2023
@OlegMoshkovich
Copy link
Member

Notes Icon does not work.

@OlegMoshkovich
Copy link
Member

Did slight adjustments to the graphics on on the about dialog.
image

@pablo-mayrgundter
Copy link
Member Author

Did slight adjustments to the graphics on on the about dialog. image

done.

@pablo-mayrgundter
Copy link
Member Author

Notes Icon does not work.

done

@pablo-mayrgundter
Copy link
Member Author

PTAL

src/Theme.jsx Outdated
MuiToggleButton: {
styleOverrides: {
sizeMedium: {
'margin': '1em',
Copy link
Member

Choose a reason for hiding this comment

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

Please change the margin to
'margin': '.2em 0em .2em 0em',

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

</DialogContent>
</MuiDialog>)
<DialogActions sx={{overflowY: 'hidden'}}>
Copy link
Member

Choose a reason for hiding this comment

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

add padding to the Dialog Actions styles.
padding: '0em 0em 2em 0em'

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Pablo Mayrgundter <pablo.mayrgundter@gmail.com>
…outControl.jsx. Add cypress scripts to package
@pablo-mayrgundter pablo-mayrgundter merged commit f43e38b into bldrs-ai:main Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Code cleanup, refactoring, style and lint
Projects
No open projects
Status: Completed
Development

Successfully merging this pull request may close these issues.

None yet

2 participants