fix: Implement DS styles and unify modals #11212

ashiskumar-1999 · 2023-09-19T17:37:08Z

Description

Implemented the styles from the DS to the Modal theme file & component. Replaced the existing modal implementation with the current Modal.

Added the DS styles from Figma in the theme file(@chakra-ui/gatsby-plugin/components/Modal.ts) & removed styles from the ModalContent in the Component file(components/Modal.tsx).
Replaced the current QuizessModal implementation with the current Modal Component.
Implemented the current Modal component in the index.tsx file instead of CodeModal component.

Preview link

https://deploy-preview-11212--ethereumorg.netlify.app/

Related Issue

Closes Modal - implement DS styles and unify modals #10749

Added the styles to the theme file aligned with the DS styles & replaced the QuizzesModal with the current customized modal component.

gatsby-cloud · 2023-09-20T08:12:26Z

✅ ethereum-org-website-dev deploy preview ready

Deploy preview
Build logs · 1h build time

…-website into dev

TylerAPfledderer · 2023-09-30T18:00:05Z

Taking a quick moment to mention that Closes #10749 should be added to the description so this PR is automatically tied to that issue.

ashiskumar-1999 · 2023-09-30T19:44:52Z

Thanks @TylerAPfledderer
Added it.

TylerAPfledderer · 2023-10-01T03:43:31Z

@ashiskumar-1999 hmmm, I believe there are two things that should be done first before this can get a proper review:

All the styling under variantCode in the component theming is related to the CodeModal component that can be seen on the home page. Head to the Site preview deploy of this PR, scroll down to the section called "A new frontier for development" and select one of the options in that list to see the modal.

Since the CodeModal is likely to be using this same general styling, I would take all of the styling from variantCode and move it to baseStyle and address the compressed width of CodeModal from there. (I'd recommend simply removing the maxWidth from the dialog part for now and use size="2xl" instead)

This would mean Modal should have no variant, and CodeModal is basically redundant against the custom Modal that has been added.

All components related to the Design System need to be added to Storybook with a stories file. So Modal.tsx needs to be converted to Modal/index.tsx and add the stories file in this new folder as Modal.stories.tsx. For the directory title the new story is housed in, it should be Molecules / Overlay Content / Modal.

For further guidance on creating this stories file and using it for visual testing, there is a doc in the project repo to view. However, I just recognized that it is a bit out-of-date. Therefore, I would view the file via a PR I just opened. Though subject to change, this would be more accurate documentation. See this commit of the file from that PR and we'll go from there.

…-website into dev

ashiskumar-1999 · 2023-10-12T16:41:02Z

Hi @TylerAPfledderer,
Addressed the above comments. Review this once in your available time.

TylerAPfledderer

I have provided suggestions to the most recent changes.

With the Storybook file, we want to favor Single-story hoisting, since there only one story expected to be used for the modal at this time. This means updating namings to import alias of the component, the story title and the single story itself.

TylerAPfledderer · 2023-10-16T15:32:01Z

src/components/Modal/Modal.stories.tsx

@@ -0,0 +1,20 @@
+import { Meta, StoryObj } from "@storybook/react"
+import Modal from "."


Suggested change

import Modal from "."

import ModalComponent from "."

TylerAPfledderer · 2023-10-16T15:33:31Z

src/components/Modal/Modal.stories.tsx

+const meta = {
+  title: "Molecules/Overlay Content/Modal/BaseModal",
+  component: Modal,
+} satisfies Meta<typeof Modal>


In addition to import alias name change.

The title should only account for the name of the component, whether or not there is just one story.

Suggested change

const meta = {

title: "Molecules/Overlay Content/Modal/BaseModal",

component: Modal,

} satisfies Meta<typeof Modal>

const meta = {

title: "Molecules/Overlay Content/Modal",

component: ModalComponent,

} satisfies Meta<typeof ModalComponent>

TylerAPfledderer · 2023-10-16T15:37:12Z

src/components/Modal/Modal.stories.tsx

+
+export default meta
+type Story = StoryObj<typeof meta>
+export const BaseModal: Story = {


Because the alias import of the component is renamed to ModalComponent and the directory path of this story file (The title in the meta object) ends with Modal, then we can give this story the same name, and Storybook will just generate the story and not a folder for it to be housed in. This tidies up the directory structure with only one story being rendered for the component.

Suggested change

export const BaseModal: Story = {

export const Modal: Story = {

You can read more here about Single-Story Hoisting

Suggested change

export const BaseModal: Story = {

export const Modal: Story = {

TylerAPfledderer · 2023-10-16T15:39:44Z

src/pages/quizzes.tsx

@@ -95,14 +96,14 @@ const QuizzesHubPage = ({ data }: PageProps<Queries.QuizzesHubPageQuery>) => {
      </Box>

      <QuizzesHubContext.Provider value={contextState}>
-        <QuizzesModal isOpen={isOpen} onClose={onClose}>
+        <Modal isOpen={isOpen} setIsOpen={onClose}>


Do not replace QuizzesModal here. Rather, update that component instead to use the custom Modal over the one imported directly from Chakra.

I think we should use directly the Modal component instead of updating the QuizzesModal. Because we have set another prop for the isOpen & setIsOpen prop of the custom Modal & we can add the value where the QuizzesModal component is being rendered. What do you think?

@ashiskumar-1999 it's more about the custom styling that is added to this instance of the modal and keeping that styling in isolation. In particular, there is conditional rendering of the background color and the use of the size prop based on screen size.

Also it would be easier to continue handling the background style in QuizzesModal than trying to do the logic in a variant for something primitive. That's not how a theme variant should be applied.

src/pages/index.tsx

TylerAPfledderer · 2023-10-16T15:42:09Z

src/@chakra-ui/gatsby-plugin/components/Modal.ts

+        /* borderTop: "1px solid",
+        borderBottom: "1px solid", */
+        padding: 0,
+        textTransform: "uppercase",


Not a part of the DS

Suggested change

textTransform: "uppercase",

TylerAPfledderer · 2023-10-16T15:48:18Z

src/@chakra-ui/gatsby-plugin/components/Modal.ts

+        fontSize: "md",
+        fontFamily: "Inter",
+      },
+      footer: {


The footer part needs it's padding updated. Currently, the ModalFooter component renders it's own padding.

Add:

px: 0, pt: "8", pb: 0,

This will create the necessary overrides here and match the DS is the extended spacing above the buttons.

pettinarip · 2024-04-23T13:04:36Z

@nloureiro PR ready for review (once the build finishes).

As discussed, we have:

on mobile, used the same styles used in the Simulator modal
unified the sizes of the modals
the close button is not absolute positioned, now it is part of the header structure
the modal padding is 32px

The affected modals and pages where you can see them are:

default Modal
- https://deploy-preview-11212--ethereumorg.netlify.app/en/developers/tutorials/
- https://deploy-preview-11212--ethereumorg.netlify.app/en/developers/docs/
CodeModal - unique modal that has its own styles
- https://deploy-preview-11212--ethereumorg.netlify.app/en/
QuizzesModal
- https://deploy-preview-11212--ethereumorg.netlify.app/en/quizzes/
SimulatorModal
- https://deploy-preview-11212--ethereumorg.netlify.app/en/wallets/

nloureiro · 2024-05-21T09:47:27Z

@pettinarip looks great! great work

I found one tiny, tiny detail that it's not a blocker. I will create an issue later on.

wackerow · 2024-05-24T21:15:41Z

@pettinarip This look ready from your perspective?

I'm noticing the content of the Quiz modal

Production (left aligned text, full width content):

PR (center aligned text, fits-to-content)

nloureiro · 2024-05-25T09:14:52Z

@pettinarip This look ready from your perspective?

I'm noticing the content of the Quiz modal

Production (left aligned text, full width content):

PR (center aligned text, fits-to-content)

From my side, it's better centered.
On the original Figma it's centered, too.

gitpoap-bot · 2024-05-29T14:29:06Z

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

Be sure to join the Ethereum.org discord if you are interested in contributing further to the project or have any questions for the team.

GitPOAP: 2024 Ethereum.org Contributor:

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

ashiskumar-1999 and others added 2 commits September 19, 2023 22:33

Fixed the Quiz Modal.

7628b33

Added the styles to the theme file aligned with the DS styles & replaced the QuizzesModal with the current customized modal component.

Merge branch 'ethereum:dev' into dev

6a067d6

github-actions bot added content 🖋️ This involves copy additions or edits review needed 👀 labels Sep 19, 2023

ashiskumar-1999 and others added 3 commits September 21, 2023 11:57

Implemented the current Modal in the CodeModal component.

bdea616

Merge branch 'ethereum:dev' into dev

d7684af

Merge branch 'dev' of https://github.com/ashiskumar-1999/ethereum-org…

56aae55

…-website into dev

ashiskumar-1999 marked this pull request as ready for review September 21, 2023 06:35

ashiskumar-1999 requested review from samajammin, wackerow, corwintines, pettinarip, minimalsm, jmcook1186 and nhsz as code owners September 21, 2023 06:35

ashiskumar-1999 changed the title ~~fix: Implement DS styles and unify modals #10749~~ fix: Implement DS styles and unify modals [ Closes issue #10749 ] Sep 21, 2023

Merge branch 'ethereum:dev' into dev

a91c18d

ashiskumar-1999 changed the title ~~fix: Implement DS styles and unify modals [ Closes issue #10749 ]~~ fix: Implement DS styles and unify modals Sep 30, 2023

ashiskumar-1999 and others added 6 commits October 3, 2023 22:39

Remove the all styling from variantCode to base style.

2827c26

Merge branch 'ethereum:dev' into dev

b6d5805

Merge branch 'dev' of https://github.com/ashiskumar-1999/ethereum-org…

ed572c2

…-website into dev

Merge branch 'ethereum:dev' into dev

03ce5dc

Stories Has been added to the StoryBook for the Modal Component.

41b2361

Merge branch 'dev' of https://github.com/ashiskumar-1999/ethereum-org…

ecc900c

…-website into dev

TylerAPfledderer suggested changes Oct 16, 2023

View reviewed changes

pettinarip marked this pull request as draft April 26, 2024 14:33

pettinarip self-assigned this Apr 26, 2024

pettinarip added 9 commits May 13, 2024 15:49

set outside as default for scrollBehavior

58c477d

update modal styles and scroll behavior

ed09fd7

Merge branch 'dev' into ashiskumar-dev

694d193

Merge branch 'dev' into ashiskumar-dev

e3d5e0f

Merge branch 'dev' into ashiskumar-dev

b8236bc

fix spacing in tutorials modal

11df7ce

override codemodal padding

4b71e5a

update QuizRadioGroup component layout and spacing

e7ab897

update Modal component styles for full size dialog

ba50c04

nloureiro approved these changes May 21, 2024

View reviewed changes

update Modal component styles for full size dialog

ad2f1d7

pettinarip marked this pull request as ready for review May 21, 2024 16:45

update padding styles for full size dialog + add story

d870d59

wackerow removed the needs review 👀 label May 22, 2024

pettinarip added 4 commits May 27, 2024 15:13

Merge branch 'dev' into ashiskumar-dev

9eecdae

Merge branch 'dev' into ashiskumar-dev

69cefbd

adjust FeedbackWidget to match the modal styles

846f422

chore: adjust FeedbackWidget width to 320px

181a754

wackerow approved these changes May 29, 2024

View reviewed changes

pettinarip approved these changes May 29, 2024

View reviewed changes

pettinarip merged commit 7039220 into ethereum:dev May 29, 2024
10 checks passed

This was referenced Jun 19, 2024

Release candidate v8.7.0 #13198

Merged

Deploy v8.7.0 #13204

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Implement DS styles and unify modals #11212

fix: Implement DS styles and unify modals #11212

ashiskumar-1999 commented Sep 19, 2023 •

edited by wackerow

Loading

gatsby-cloud bot commented Sep 20, 2023 •

edited

Loading

TylerAPfledderer commented Sep 30, 2023

ashiskumar-1999 commented Sep 30, 2023

TylerAPfledderer commented Oct 1, 2023 •

edited

Loading

ashiskumar-1999 commented Oct 12, 2023

TylerAPfledderer left a comment

TylerAPfledderer Oct 16, 2023

TylerAPfledderer Oct 16, 2023

TylerAPfledderer Oct 16, 2023

TylerAPfledderer Oct 16, 2023

ashiskumar-1999 Oct 25, 2023

TylerAPfledderer Nov 15, 2023

TylerAPfledderer Oct 16, 2023

TylerAPfledderer Oct 16, 2023

pettinarip commented Apr 23, 2024

nloureiro commented May 21, 2024

wackerow commented May 24, 2024

nloureiro commented May 25, 2024

gitpoap-bot bot commented May 29, 2024

		@@ -0,0 +1,20 @@
		import { Meta, StoryObj } from "@storybook/react"
		import Modal from "."

	export const BaseModal: Story = {
	export const Modal: Story = {

fix: Implement DS styles and unify modals #11212

fix: Implement DS styles and unify modals #11212

Conversation

ashiskumar-1999 commented Sep 19, 2023 • edited by wackerow Loading

Description

Preview link

Related Issue

gatsby-cloud bot commented Sep 20, 2023 • edited Loading

✅ ethereum-org-website-dev deploy preview ready

TylerAPfledderer commented Sep 30, 2023

ashiskumar-1999 commented Sep 30, 2023

TylerAPfledderer commented Oct 1, 2023 • edited Loading

ashiskumar-1999 commented Oct 12, 2023

TylerAPfledderer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pettinarip commented Apr 23, 2024

nloureiro commented May 21, 2024

wackerow commented May 24, 2024

nloureiro commented May 25, 2024

gitpoap-bot bot commented May 29, 2024

ashiskumar-1999 commented Sep 19, 2023 •

edited by wackerow

Loading

gatsby-cloud bot commented Sep 20, 2023 •

edited

Loading

TylerAPfledderer commented Oct 1, 2023 •

edited

Loading