From fa5b319c4c13306ff8fb820796bbe6dd9bb56f3f Mon Sep 17 00:00:00 2001 From: jonathan santos da silva Date: Fri, 27 Sep 2024 07:40:46 -0300 Subject: [PATCH 1/7] fix: add class on mount and fix backdrop --- packages/core/src/components/modal/modal.scss | 2 ++ packages/core/src/components/modal/modal.tsx | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/core/src/components/modal/modal.scss b/packages/core/src/components/modal/modal.scss index 34a1dbe51..2fa0c9766 100644 --- a/packages/core/src/components/modal/modal.scss +++ b/packages/core/src/components/modal/modal.scss @@ -1,6 +1,8 @@ @import '~@atomium/scss-utils/index'; .atom-modal { + position: fixed; + &__close { position: absolute; right: var(--spacing-small); diff --git a/packages/core/src/components/modal/modal.tsx b/packages/core/src/components/modal/modal.tsx index 42a0805fd..7a3abc475 100644 --- a/packages/core/src/components/modal/modal.tsx +++ b/packages/core/src/components/modal/modal.tsx @@ -28,6 +28,7 @@ export class AtomModal { @Prop() hasFooter = true @Prop() disablePrimary = false @Prop() disableSecondary = false + @Prop() isOpen = false @Event() atomCloseClick: EventEmitter @Event() atomDidDismiss: EventEmitter @@ -49,7 +50,7 @@ export class AtomModal { } componentDidLoad() { - document.body.classList.remove(BACKDROP_NO_SCROLL) + document.body.classList.add(BACKDROP_NO_SCROLL) this.modal.close = async () => { await this.modal.dismiss() @@ -95,6 +96,7 @@ export class AtomModal { }} onIonModalDidDismiss={this.handleDidDimiss} onDidPresent={this.handleDidPresent} + isOpen={this.isOpen} >
{iconType && ( From fb57b25a040286a52c886dd9c719a01e7b6126de Mon Sep 17 00:00:00 2001 From: jowjow22 Date: Fri, 27 Sep 2024 10:16:32 -0300 Subject: [PATCH 2/7] chore: use readonly on properties --- packages/core/src/components/modal/modal.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/core/src/components/modal/modal.tsx b/packages/core/src/components/modal/modal.tsx index 7a3abc475..4649e1b55 100644 --- a/packages/core/src/components/modal/modal.tsx +++ b/packages/core/src/components/modal/modal.tsx @@ -38,7 +38,7 @@ export class AtomModal { private modal: HTMLAtomModalElement - private alertMap: AlertType = { + private readonly alertMap: AlertType = { alert: { icon: 'alert-outline', color: 'warning', @@ -59,24 +59,24 @@ export class AtomModal { } } - private handleDidDimiss = () => { + private readonly handleDidDimiss = () => { this.atomDidDismiss.emit(this.modal) } - private handleDidPresent = () => { + private readonly handleDidPresent = () => { this.atomDidPresent.emit(this.modal) } - private handleCloseClick = async () => { + private readonly handleCloseClick = async () => { this.atomCloseClick.emit(this.modal) this.modal.close() } - private handleSecondaryClick = () => { + private readonly handleSecondaryClick = () => { this.atomSecondaryClick.emit(this.modal) } - private handlePrimaryClick = () => { + private readonly handlePrimaryClick = () => { this.atomPrimaryClick.emit(this.modal) } From 998eb81fad7911d33580da75138ef960c665696a Mon Sep 17 00:00:00 2001 From: jowjow22 Date: Fri, 27 Sep 2024 14:00:41 -0300 Subject: [PATCH 3/7] test: isopen prop --- packages/core/src/components.d.ts | 2 ++ .../core/src/components/modal/modal.spec.ts | 26 +++++++++++++++++++ packages/core/src/components/modal/modal.tsx | 19 ++++++++++++-- .../components/modal/stories/modal.args.ts | 9 +++++++ .../modal/stories/modal.core.stories.tsx | 1 + .../modal/stories/modal.react.stories.tsx | 1 + .../modal/stories/modal.vue.stories.tsx | 1 + 7 files changed, 57 insertions(+), 2 deletions(-) diff --git a/packages/core/src/components.d.ts b/packages/core/src/components.d.ts index fc758be82..b27df1b17 100644 --- a/packages/core/src/components.d.ts +++ b/packages/core/src/components.d.ts @@ -165,6 +165,7 @@ export namespace Components { "hasDivider": boolean; "hasFooter": boolean; "headerTitle": string; + "isOpen": boolean; "primaryText"?: string; "progress"?: number; "secondaryText"?: string; @@ -703,6 +704,7 @@ declare namespace LocalJSX { "hasDivider"?: boolean; "hasFooter"?: boolean; "headerTitle"?: string; + "isOpen"?: boolean; "onAtomCloseClick"?: (event: AtomModalCustomEvent) => void; "onAtomDidDismiss"?: (event: AtomModalCustomEvent) => void; "onAtomDidPresent"?: (event: AtomModalCustomEvent) => void; diff --git a/packages/core/src/components/modal/modal.spec.ts b/packages/core/src/components/modal/modal.spec.ts index d00e18c2a..85e0599f2 100644 --- a/packages/core/src/components/modal/modal.spec.ts +++ b/packages/core/src/components/modal/modal.spec.ts @@ -69,6 +69,32 @@ describe('atom-modal', () => { expect(page.root?.textContent).toContain('Header from prop') }) + it('should render modal closed if is open is set to false', async () => { + page = await newSpecPage({ + components: [AtomModal], + html: ` + + Modal content +
Custom Header
+
+ `, + }) + + expect(page.root?.innerHTML).not.toContain('isopen') + }) + it('should render modal opened if is open is set to true', async () => { + page = await newSpecPage({ + components: [AtomModal], + html: ` + + Modal content +
Custom Header
+
+ `, + }) + + expect(page.root?.innerHTML).toContain('isopen') + }) it('should render icon type when alertType is passed', async () => { await page.setContent(` diff --git a/packages/core/src/components/modal/modal.tsx b/packages/core/src/components/modal/modal.tsx index 4649e1b55..a6d55d4e1 100644 --- a/packages/core/src/components/modal/modal.tsx +++ b/packages/core/src/components/modal/modal.tsx @@ -1,4 +1,12 @@ -import { Component, Event, EventEmitter, Host, Prop, h } from '@stencil/core' +import { + Component, + Event, + EventEmitter, + Host, + Prop, + Watch, + h, +} from '@stencil/core' import { IconProps } from '../../icons' @@ -36,6 +44,13 @@ export class AtomModal { @Event() atomPrimaryClick: EventEmitter @Event() atomSecondaryClick: EventEmitter + @Watch('isOpen') + watchPropHandler(newValue: boolean) { + if (newValue) { + document.body.classList.add(BACKDROP_NO_SCROLL) + } + } + private modal: HTMLAtomModalElement private readonly alertMap: AlertType = { @@ -50,7 +65,7 @@ export class AtomModal { } componentDidLoad() { - document.body.classList.add(BACKDROP_NO_SCROLL) + document.body.classList.remove(BACKDROP_NO_SCROLL) this.modal.close = async () => { await this.modal.dismiss() diff --git a/packages/core/src/components/modal/stories/modal.args.ts b/packages/core/src/components/modal/stories/modal.args.ts index 5c58e274b..b2801f383 100644 --- a/packages/core/src/components/modal/stories/modal.args.ts +++ b/packages/core/src/components/modal/stories/modal.args.ts @@ -80,6 +80,14 @@ export const ModalStoryArgs = { category: Category.PROPERTIES, }, }, + isOpen: { + control: 'boolean', + description: 'If true, the modal will be opened. Default is false', + table: { + category: Category.PROPERTIES, + }, + defaultValue: false, + }, disableSecondary: { control: 'boolean', description: @@ -161,4 +169,5 @@ export const ModalComponentArgs = { hasDivider: false, disableSecondary: false, disablePrimary: false, + isOpen: false, } diff --git a/packages/core/src/components/modal/stories/modal.core.stories.tsx b/packages/core/src/components/modal/stories/modal.core.stories.tsx index 8ef8864aa..039bcface 100644 --- a/packages/core/src/components/modal/stories/modal.core.stories.tsx +++ b/packages/core/src/components/modal/stories/modal.core.stories.tsx @@ -23,6 +23,7 @@ const createModal = (args) => { header-title="${args.headerTitle}" disable-secondary="${args.disableSecondary}" disable-primary="${args.disablePrimary}" + is-open="${args.isOpen}" >
Custom Header

Modal Content

diff --git a/packages/core/src/components/modal/stories/modal.react.stories.tsx b/packages/core/src/components/modal/stories/modal.react.stories.tsx index 98b39f102..2d9172033 100644 --- a/packages/core/src/components/modal/stories/modal.react.stories.tsx +++ b/packages/core/src/components/modal/stories/modal.react.stories.tsx @@ -22,6 +22,7 @@ const createModal = (args) => ( progress={args.progress} disablePrimary={args.disablePrimary} disableSecondary={args.disableSecondary} + isOpen={args.isOpen} >
Custom Header

Modal Content

diff --git a/packages/core/src/components/modal/stories/modal.vue.stories.tsx b/packages/core/src/components/modal/stories/modal.vue.stories.tsx index 20da66fb4..196ae3e71 100644 --- a/packages/core/src/components/modal/stories/modal.vue.stories.tsx +++ b/packages/core/src/components/modal/stories/modal.vue.stories.tsx @@ -25,6 +25,7 @@ const createModal = (args, themeColor = 'light') => ({ progress="${args.progress}" disable-primary="${args.disablePrimary}" disable-secondary="${args.disableSecondary}" + is-open="${args.isOpen}" > {{ args.label }} From 3839dd535aaad3f0db99f17e9b9224e816bc9067 Mon Sep 17 00:00:00 2001 From: jowjow22 Date: Fri, 27 Sep 2024 14:30:55 -0300 Subject: [PATCH 4/7] test: watchers on isopen --- .../core/src/components/modal/modal.spec.ts | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/core/src/components/modal/modal.spec.ts b/packages/core/src/components/modal/modal.spec.ts index 85e0599f2..dc573cb59 100644 --- a/packages/core/src/components/modal/modal.spec.ts +++ b/packages/core/src/components/modal/modal.spec.ts @@ -2,6 +2,13 @@ import { newSpecPage, SpecPage } from '@stencil/core/testing' import { AtomModal } from './modal' +Object.defineProperty(document.body, 'classList', { + value: { + add: jest.fn(), + remove: jest.fn(), + }, +}) + describe('atom-modal', () => { let page: SpecPage @@ -94,6 +101,41 @@ describe('atom-modal', () => { }) expect(page.root?.innerHTML).toContain('isopen') + + page.rootInstance.isOpen = false + + await page.waitForChanges() + + expect(document.body.classList.remove).toHaveBeenCalled() + }) + it('should call remove and add on backdrop no scroll class when is-open changes', async () => { + page = await newSpecPage({ + components: [AtomModal], + html: ` + + Modal content +
Custom Header
+
+ `, + }) + + expect(page.root?.innerHTML).toContain('isopen') + + page.rootInstance.isOpen = false + + await page.waitForChanges() + + expect(document.body.classList.remove).toHaveBeenCalledWith( + 'backdrop-no-scroll' + ) + + page.rootInstance.isOpen = true + + await page.waitForChanges() + + expect(document.body.classList.add).toHaveBeenCalledWith( + 'backdrop-no-scroll' + ) }) it('should render icon type when alertType is passed', async () => { await page.setContent(` From 8f189042b308f538e38389a74c164ac25af7ce2d Mon Sep 17 00:00:00 2001 From: jowjow22 Date: Fri, 27 Sep 2024 16:41:24 -0300 Subject: [PATCH 5/7] fix: create class to html too --- README.md | 4 +-- packages/core/src/components/modal/modal.tsx | 27 ++++++-------------- packages/core/src/global/global.scss | 4 +++ 3 files changed, 14 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index f532e07b9..7ce089131 100644 --- a/README.md +++ b/README.md @@ -130,11 +130,11 @@ To locally test Atomium using Alpha/Beta versions, follow the steps below: 1. Update the `.release-please-manifest.json` file in the root directory of the Atomium project with the next version number + alpha. Ex: the current version is `2.10.0`, so the next alpha version can be `2.11.0-alpha.1` (OBS: in the example, it is updating only the core lib, update the libs that your changes impact). -![add alpha version to release](https://github.com/user-attachments/assets/3e3adaa2-1bcd-4442-8d1a-118cc14cc274) +![add alpha version to release](https://github.com/user-attachments/assets/91418116-266c-45f1-9cf2-bdf2d6c1a7eb) 2. Add the same version to the repespective `package.json` file in the root directory of the lib project. Ex: packages/core/package.json -![add alpha to package](https://github.com/user-attachments/assets/3e3adaa2-1bcd-4442-8d1a-118cc14cc274) +![add alpha to package](https://github.com/user-attachments/assets/ec4bfbf2-a822-4cf5-b7c4-66575fd36230) 3. Build the Atomium libraries by running the following command in the root directory of the Atomium project diff --git a/packages/core/src/components/modal/modal.tsx b/packages/core/src/components/modal/modal.tsx index a6d55d4e1..d3f9e0be7 100644 --- a/packages/core/src/components/modal/modal.tsx +++ b/packages/core/src/components/modal/modal.tsx @@ -1,20 +1,9 @@ -import { - Component, - Event, - EventEmitter, - Host, - Prop, - Watch, - h, -} from '@stencil/core' +import { Component, Event, EventEmitter, Host, Prop, h } from '@stencil/core' import { IconProps } from '../../icons' type AlertType = Record<'alert' | 'error', { icon: IconProps; color: string }> -/* @todo it's needed to prevent a ionic error. In the version 8.0 it was fixed, remove it after the upgrade. - * https://github.com/ionic-team/ionic-framework/issues/23942 - */ const BACKDROP_NO_SCROLL = 'backdrop-no-scroll' type HTMLAtomModalElement = HTMLIonModalElement & { close: () => Promise } @@ -44,13 +33,6 @@ export class AtomModal { @Event() atomPrimaryClick: EventEmitter @Event() atomSecondaryClick: EventEmitter - @Watch('isOpen') - watchPropHandler(newValue: boolean) { - if (newValue) { - document.body.classList.add(BACKDROP_NO_SCROLL) - } - } - private modal: HTMLAtomModalElement private readonly alertMap: AlertType = { @@ -65,12 +47,16 @@ export class AtomModal { } componentDidLoad() { + /* @todo it's needed to prevent a ionic error. In the version 8.0 it was fixed, remove it after the upgrade. + * https://github.com/ionic-team/ionic-framework/issues/23942 + */ document.body.classList.remove(BACKDROP_NO_SCROLL) this.modal.close = async () => { await this.modal.dismiss() document.body.classList.remove(BACKDROP_NO_SCROLL) + document.documentElement.classList.remove(BACKDROP_NO_SCROLL) } } @@ -80,6 +66,9 @@ export class AtomModal { private readonly handleDidPresent = () => { this.atomDidPresent.emit(this.modal) + + document.body.classList.add(BACKDROP_NO_SCROLL) + document.documentElement.classList.add(BACKDROP_NO_SCROLL) } private readonly handleCloseClick = async () => { diff --git a/packages/core/src/global/global.scss b/packages/core/src/global/global.scss index 3ac44b6ac..29f9b2809 100644 --- a/packages/core/src/global/global.scss +++ b/packages/core/src/global/global.scss @@ -111,3 +111,7 @@ Issue: https://github.com/ionic-team/ionic-framework/issues/27500 } } } + +html.backdrop-no-scroll { + overflow: hidden; +} \ No newline at end of file From 52b90dba9b973366411dccf545a270fd2c7e2f1a Mon Sep 17 00:00:00 2001 From: jowjow22 Date: Mon, 30 Sep 2024 14:08:25 -0300 Subject: [PATCH 6/7] fix: call close on dismiss --- .../core/src/components/modal/modal.spec.ts | 29 +++++++++++------ packages/core/src/components/modal/modal.tsx | 32 +++++++++++++------ 2 files changed, 41 insertions(+), 20 deletions(-) diff --git a/packages/core/src/components/modal/modal.spec.ts b/packages/core/src/components/modal/modal.spec.ts index dc573cb59..4edceb367 100644 --- a/packages/core/src/components/modal/modal.spec.ts +++ b/packages/core/src/components/modal/modal.spec.ts @@ -9,6 +9,13 @@ Object.defineProperty(document.body, 'classList', { }, }) +Object.defineProperty(document.documentElement, 'classList', { + value: { + add: jest.fn(), + remove: jest.fn(), + }, +}) + describe('atom-modal', () => { let page: SpecPage @@ -121,22 +128,24 @@ describe('atom-modal', () => { expect(page.root?.innerHTML).toContain('isopen') - page.rootInstance.isOpen = false + page.rootInstance.modal = { + dismiss: jest.fn(), + close: jest.fn(), + } + page.rootInstance.handleDidPresent() - await page.waitForChanges() + expect(document.body.classList.add).toHaveBeenCalled() + expect(document.documentElement.classList.add).toHaveBeenCalled() - expect(document.body.classList.remove).toHaveBeenCalledWith( - 'backdrop-no-scroll' - ) + page.rootInstance.handleDidDismiss() - page.rootInstance.isOpen = true + expect(page.rootInstance.modal.close).toHaveBeenCalled() - await page.waitForChanges() + page.rootInstance.handleCloseClick() - expect(document.body.classList.add).toHaveBeenCalledWith( - 'backdrop-no-scroll' - ) + expect(page.rootInstance.modal.close).toHaveBeenCalled() }) + it('should render icon type when alertType is passed', async () => { await page.setContent(` diff --git a/packages/core/src/components/modal/modal.tsx b/packages/core/src/components/modal/modal.tsx index d3f9e0be7..4b32552e7 100644 --- a/packages/core/src/components/modal/modal.tsx +++ b/packages/core/src/components/modal/modal.tsx @@ -6,7 +6,9 @@ type AlertType = Record<'alert' | 'error', { icon: IconProps; color: string }> const BACKDROP_NO_SCROLL = 'backdrop-no-scroll' -type HTMLAtomModalElement = HTMLIonModalElement & { close: () => Promise } +export type HTMLAtomModalElement = HTMLIonModalElement & { + close: () => Promise +} @Component({ tag: 'atom-modal', @@ -25,7 +27,7 @@ export class AtomModal { @Prop() hasFooter = true @Prop() disablePrimary = false @Prop() disableSecondary = false - @Prop() isOpen = false + @Prop({ mutable: true }) isOpen = false @Event() atomCloseClick: EventEmitter @Event() atomDidDismiss: EventEmitter @@ -46,6 +48,16 @@ export class AtomModal { }, } + private readonly addClasses = () => { + document.body.classList.add(BACKDROP_NO_SCROLL) + document.documentElement.classList.add(BACKDROP_NO_SCROLL) + } + + private readonly removeClasses = () => { + document.body.classList.remove(BACKDROP_NO_SCROLL) + document.documentElement.classList.remove(BACKDROP_NO_SCROLL) + } + componentDidLoad() { /* @todo it's needed to prevent a ionic error. In the version 8.0 it was fixed, remove it after the upgrade. * https://github.com/ionic-team/ionic-framework/issues/23942 @@ -54,21 +66,20 @@ export class AtomModal { this.modal.close = async () => { await this.modal.dismiss() - - document.body.classList.remove(BACKDROP_NO_SCROLL) - document.documentElement.classList.remove(BACKDROP_NO_SCROLL) + this.removeClasses() + this.isOpen = false } } - private readonly handleDidDimiss = () => { + private readonly handleDidDismiss = () => { this.atomDidDismiss.emit(this.modal) + this.modal.close() } private readonly handleDidPresent = () => { this.atomDidPresent.emit(this.modal) - - document.body.classList.add(BACKDROP_NO_SCROLL) - document.documentElement.classList.add(BACKDROP_NO_SCROLL) + this.isOpen = true + this.addClasses() } private readonly handleCloseClick = async () => { @@ -98,7 +109,8 @@ export class AtomModal { 'atom-modal': true, 'atom-modal--progress': !!this.progress, }} - onIonModalDidDismiss={this.handleDidDimiss} + onIonModalDidDismiss={this.handleDidDismiss} + onDidDismiss={this.handleDidDismiss} onDidPresent={this.handleDidPresent} isOpen={this.isOpen} > From 30975b10e5a140022926d67bd0a6c3badd6faf1b Mon Sep 17 00:00:00 2001 From: jowjow22 Date: Mon, 30 Sep 2024 14:29:40 -0300 Subject: [PATCH 7/7] test: improve tests --- .../core/src/components/modal/modal.spec.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/core/src/components/modal/modal.spec.ts b/packages/core/src/components/modal/modal.spec.ts index 4edceb367..a66ecbcc1 100644 --- a/packages/core/src/components/modal/modal.spec.ts +++ b/packages/core/src/components/modal/modal.spec.ts @@ -146,6 +146,51 @@ describe('atom-modal', () => { expect(page.rootInstance.modal.close).toHaveBeenCalled() }) + it('should call remove and add on backdrop no scroll class when is-open changes', async () => { + page = await newSpecPage({ + components: [AtomModal], + html: ` + + Modal content +
Custom Header
+
+ `, + }) + + expect(page.root?.innerHTML).toContain('isopen') + + page.rootInstance.modal = { + dismiss: jest.fn(), + } + + page.rootInstance.handleDidPresent() + + expect(document.body.classList.add).toHaveBeenCalled() + expect(document.documentElement.classList.add).toHaveBeenCalled() + }) + + it('should call remove on class list when call remove classes', async () => { + page = await newSpecPage({ + components: [AtomModal], + html: ` + + Modal content +
Custom Header
+
+ `, + }) + + page.rootInstance.addClasses() + + expect(document.body.classList.add).toHaveBeenCalled() + expect(document.documentElement.classList.add).toHaveBeenCalled() + + page.rootInstance.removeClasses() + + expect(document.body.classList.remove).toHaveBeenCalled() + expect(document.documentElement.classList.remove).toHaveBeenCalled() + }) + it('should render icon type when alertType is passed', async () => { await page.setContent(`