SSD-805: ta-dialog#347
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This pull request adds a new ta-dialog component to the Transistory Assistance (CSS) documentation system. The component provides HTML/CSS-based modal dialog functionality for MYDS-compliant applications that don't use React, featuring both light and dark theme support.
Changes:
- Added new dialog component with CSS styling and dark mode support
- Created comprehensive documentation in both English and Malay
- Added HTML preview file demonstrating dialog functionality with native dialog API
Reviewed changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
apps/docs/public/assets/ta-preview/dialog/ta-dialog-preview.html |
HTML preview demonstrating the dialog component with JavaScript for open/close/backdrop-click functionality |
apps/docs/public/assets/ta-preview/dialog/dialog.css |
Main CSS file defining dialog styles with myds-prefixed class names |
apps/docs/public/assets/ta-preview/dialog/dialog-dark.css |
Dark theme CSS overrides for the dialog component |
apps/docs/content/docs/develop/(transistory-assistance)/ta-dialog.mdx |
English documentation with preview tabs, setup instructions, and API reference |
apps/docs/content/docs/develop/(transistory-assistance)/ta-dialog.ms.mdx |
Malay translation of the dialog documentation |
apps/docs/content/docs/develop/meta.en.json |
Formatting-only changes (whitespace standardization) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button class="button-myds-primary" onclick=openDialog()> | ||
| Default | ||
| </button> | ||
|
|
||
| <dialog> | ||
| <div class="myds-dialog-header"> | ||
| <h2 class="myds-dialog-title">Dialog Title</h2> | ||
| </div> | ||
|
|
||
| <div class="myds-dialog-content"> | ||
| <p>Dialog content goes here.</p> | ||
| </div> | ||
|
|
||
| <div class="myds-dialog-footer"> | ||
| <div class="myds-dialog-actions-group"> | ||
| <button class="button-myds button-myds-secondary" type="button" onclick=closeDialog() autofocus> | ||
| Secondary Action | ||
| </button> | ||
| <button class="button-myds button-myds-primary" type="button" onclick=closeDialog()> | ||
| Primary Action | ||
| </button> | ||
| </div> | ||
| </div> | ||
|
|
||
| <button class="myds-dialog-close" onclick="closeDialog()" aria-label="Close"> |
There was a problem hiding this comment.
The onclick attribute values are inconsistently quoted. Lines 21, 36, and 39 use onclick=openDialog() and onclick=closeDialog() without quotes, while line 45 uses onclick="closeDialog()" with quotes. For consistency and best practices, all onclick attributes should have their values quoted.
| <Tab value="DialogCSS"> | ||
| ```css | ||
| /* Base Styles */ | ||
| html, | ||
| body { | ||
| height: 100%; | ||
| background-color: white; | ||
| } | ||
|
|
||
| .dark body { | ||
| background-color: #161619; | ||
| } | ||
|
|
||
| /* Trigger Button */ | ||
| .button-myds-primary { | ||
| padding: 0.5rem 1rem; | ||
| border-radius: 0.375rem; | ||
| border: 1px solid #2563eb; | ||
| background-color: #2563eb; | ||
| color: #ffffff; | ||
| font-weight: 500; | ||
| cursor: pointer; | ||
| transition: all 150ms; | ||
| } | ||
|
|
||
| .button-myds-primary:hover { | ||
| background-color: #1d4ed8; | ||
| } | ||
|
|
||
| .button-myds-primary:focus { | ||
| outline: 3px solid #96b7ff66; | ||
| } | ||
|
|
||
| .page-center { | ||
| display: grid; | ||
| place-content: center; | ||
| min-height: 100%; | ||
| } | ||
|
|
||
| /* Dialog Base */ | ||
| dialog { | ||
| position: fixed; | ||
| left: 50%; | ||
| top: 50%; | ||
| transform: translate(-50%, -50%); | ||
| width: 100%; | ||
| min-width: 300px; | ||
| max-width: calc(100dvw - 36px); | ||
| box-sizing: border-box; | ||
| display: flex; | ||
| flex-direction: column; | ||
| align-items: flex-start; | ||
| border-radius: 0.5rem; | ||
| box-shadow: 0 10px 15px -3px rgb(0 0 0 / 0.1), 0 4px 6px -4px rgb(0 0 0 / 0.1); | ||
| border: 1px solid #e5e7eb; | ||
| background-color: #ffffff; | ||
| padding: 0; | ||
| z-index: 1000; | ||
| opacity: 0; | ||
| transition: opacity 0.2s ease-out, transform 0.2s ease-out; | ||
| } | ||
|
|
||
| dialog:not([open]) { | ||
| display: none; | ||
| } | ||
|
|
||
| dialog[open] { | ||
| opacity: 1; | ||
| animation: dialog-slide-up 0.2s ease-out; | ||
| } | ||
|
|
||
| @keyframes dialog-slide-up { | ||
| from { | ||
| opacity: 0; | ||
| transform: translate(-50%, -48%) scale(0.95); | ||
| } | ||
| to { | ||
| opacity: 1; | ||
| transform: translate(-50%, -50%) scale(1); | ||
| } | ||
| } | ||
|
|
||
| /* Backdrop */ | ||
| dialog::backdrop { | ||
| position: fixed; | ||
| inset: 0; | ||
| z-index: 900; | ||
| height: 100%; | ||
| width: 100%; | ||
| background-color: #3f3f4699; | ||
| opacity: 0; | ||
| animation: backdrop-fade-in 0.15s ease-out forwards; | ||
| } | ||
|
|
||
| @keyframes backdrop-fade-in { | ||
| from { | ||
| opacity: 0; | ||
| } | ||
| to { | ||
| opacity: 1; | ||
| } | ||
| } | ||
|
|
||
| @media (min-width: 640px) { | ||
| dialog { | ||
| max-width: 32rem; | ||
| } | ||
| } | ||
|
|
||
| /* Dialog Close Button */ | ||
| .dialog-close { | ||
| position: absolute; | ||
| right: 1rem; | ||
| top: 1rem; | ||
| display: grid; | ||
| place-content: center; | ||
| aspect-ratio: 1; | ||
| border-radius: 0.375rem; | ||
| padding: 0.5rem; | ||
| user-select: none; | ||
| font-weight: 500; | ||
| outline: none; | ||
| transition: all 150ms; | ||
| cursor: pointer; | ||
| text-align: center; | ||
| background-color: #ffffff; | ||
| border: 1px solid #e5e7eb; | ||
| color: #111827; | ||
| box-shadow: 0 1px 2px 0 rgb(0 0 0 / 0.05); | ||
| } | ||
|
|
||
| .dialog-close:hover { | ||
| background-color: #f9fafb; | ||
| border-color: #d1d5db; | ||
| color: #111827; | ||
| } | ||
|
|
||
| .dialog-close:active { | ||
| transform: translateY(0.5px); | ||
| } | ||
|
|
||
| .dialog-close:focus { | ||
| outline: none; | ||
| box-shadow: 0 0 0 3px #96b7ff66; | ||
| } | ||
|
|
||
| .dialog-close:disabled { | ||
| cursor: not-allowed; | ||
| pointer-events: none; | ||
| background-color: #f3f4f6; | ||
| color: #9ca3af; | ||
| border-color: transparent; | ||
| } | ||
|
|
||
| .dialog-close svg { | ||
| width: 1.25rem; | ||
| height: 1.25rem; | ||
| } | ||
|
|
||
| /* Dialog Header */ | ||
| .dialog-header { | ||
| width: 100%; | ||
| padding: 1.5rem 1.5rem 0.5rem; | ||
| } | ||
|
|
||
| .dialog-title { | ||
| font-size: 1.125rem; | ||
| line-height: 1.75rem; | ||
| color: #111827; | ||
| font-weight: 600; | ||
| } | ||
|
|
||
| /* Dialog Content */ | ||
| .dialog-content { | ||
| width: 100%; | ||
| padding: 0 1.5rem; | ||
| color: #6b7280; | ||
| font-size: 0.875rem; | ||
| line-height: 1.25rem; | ||
| } | ||
|
|
||
| /* Dialog Footer */ | ||
| .dialog-footer { | ||
| display: flex; | ||
| width: 100%; | ||
| padding: 1.5rem; | ||
| gap: 0.75rem; | ||
| flex-direction: column; | ||
| align-items: flex-end; | ||
| justify-content: space-between; | ||
| } | ||
|
|
||
| .dialog-actions-group { | ||
| display: flex; | ||
| gap: 0.75rem; | ||
| justify-content: flex-end; | ||
| flex-direction: column; | ||
| flex-grow: 1; | ||
| width: 100%; | ||
| } | ||
|
|
||
| .dialog-actions-group > * { | ||
| width: 100%; | ||
| place-content: center; | ||
| } | ||
|
|
||
| /* Dialog Action Buttons - MYDS Components */ | ||
| .button-myds { | ||
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| gap: 0.375rem; | ||
| border-radius: 0.375rem; | ||
| width: 100%; | ||
| user-select: none; | ||
| font-weight: 500; | ||
| outline: none; | ||
| transition: all 150ms; | ||
| cursor: pointer; | ||
| text-align: center; | ||
| padding: 0.375rem 0.625rem; | ||
| font-size: 0.875rem; | ||
| line-height: 1.25rem; | ||
| border: 1px solid transparent; | ||
| } | ||
|
|
||
| .button-myds:disabled { | ||
| cursor: not-allowed; | ||
| } | ||
|
|
||
| .button-myds:active { | ||
| transform: translateY(0.5px); | ||
| } | ||
|
|
||
| .button-myds:focus { | ||
| outline: 3px solid #96b7ff66; | ||
| } | ||
|
|
||
| .button-myds-secondary { | ||
| background-color: #ffffff; | ||
| border-color: #e4e4e7; | ||
| color: #3f3f46; | ||
| } | ||
|
|
||
| .button-myds-secondary:hover { | ||
| background-color: #fafafa; | ||
| } | ||
|
|
||
| .button-myds-secondary:disabled { | ||
| background-color: #f4f4f566; | ||
| color: #52525b66; | ||
| border: none; | ||
| pointer-events: none; | ||
| } | ||
|
|
||
| .button-myds-primary { | ||
| background-color: #2563eb; | ||
| border-color: #2563eb; | ||
| color: #ffffff; | ||
| } | ||
|
|
||
| .button-myds-primary:hover { | ||
| background-color: #1d4ed8; | ||
| } | ||
|
|
||
| .button-myds-primary:disabled { | ||
| background-color: #c2d5ff; | ||
| color: #ffffff66; | ||
| border: none; | ||
| pointer-events: none; | ||
| } | ||
|
|
||
| @media (min-width: 640px) { | ||
| .dialog-footer { | ||
| flex-direction: row; | ||
| justify-content: flex-end; | ||
| } | ||
|
|
||
| .dialog-actions-group { | ||
| flex-direction: row; | ||
| width: auto; | ||
| } | ||
|
|
||
| .dialog-actions-group > *, | ||
| .button-myds { | ||
| width: auto; | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The DialogCSS tab documentation shows incorrect class names that don't match the actual CSS implementation. The documentation shows non-prefixed class names like .page-center, .dialog-close, .dialog-header, .dialog-title, .dialog-content, .dialog-footer, and .dialog-actions-group, but the actual CSS file (dialog.css) uses myds- prefixed versions: .myds-page-center, .myds-dialog-close, .myds-dialog-header, .myds-dialog-title, .myds-dialog-content, .myds-dialog-footer, and .myds-dialog-actions-group. This documentation will mislead developers trying to use these components.
| | Selector | Purpose | | ||
| | ------------------------ | ------------------------------------ | | ||
| | `dialog` | Native HTML dialog element with custom styling | | ||
| | `dialog::backdrop` | The backdrop overlay (styled via pseudo-element) | | ||
| | `.dialog-header` | Header section | | ||
| | `.dialog-title` | Dialog title styling | | ||
| | `.dialog-content` | Content area | | ||
| | `.dialog-footer` | Footer section | | ||
| | `.dialog-actions-group` | Wrapper for footer buttons | | ||
| | `.dialog-close` | Close button | | ||
| | `.button-myds` | Base button styles | | ||
| | `.button-myds-primary` | Primary action button | | ||
| | `.button-myds-secondary` | Secondary action button | | ||
| | `.page-center` | Centers content on the page | |
There was a problem hiding this comment.
The Anatomy table lists incorrect selectors that don't match the actual CSS implementation. The table shows .dialog-header, .dialog-title, .dialog-content, .dialog-footer, .dialog-actions-group, .dialog-close, and .page-center, but the actual CSS uses myds- prefixed versions for all of these except button-related classes.
| | Pemilih | Tujuan | | ||
| | ------------------------ | ------------------------------------ | | ||
| | `dialog` | Elemen dialog HTML asli dengan penggayaan tersuai | | ||
| | `dialog::backdrop` | Lapisan latar belakang (digayakan melalui pseudo-element) | | ||
| | `.dialog-header` | Bahagian pengepala | | ||
| | `.dialog-title` | Penggayaan tajuk dialog | | ||
| | `.dialog-content` | Kawasan kandungan | | ||
| | `.dialog-footer` | Bahagian pengaki | | ||
| | `.dialog-actions-group` | Pembungkus untuk butang pengaki | | ||
| | `.dialog-close` | Butang tutup | | ||
| | `.button-myds` | Gaya butang asas | | ||
| | `.button-myds-primary` | Butang tindakan utama | | ||
| | `.button-myds-secondary` | Butang tindakan sekunder | | ||
| | `.page-center` | Memusatkan kandungan pada halaman | |
There was a problem hiding this comment.
The Anatomy table lists incorrect selectors that don't match the actual CSS implementation. The table shows .dialog-header, .dialog-title, .dialog-content, .dialog-footer, .dialog-actions-group, .dialog-close, and .page-center, but the actual CSS uses myds- prefixed versions for all of these except button-related classes.
| - **Dialog Element**: Native HTML5 dialog element | ||
| - **Dialog Header**: Contains the title | ||
| - **Dialog Content**: Main content area | ||
| - **Dialog Footer**: Contains action buttons wrapped in `dialog-actions-group` |
There was a problem hiding this comment.
The text mentions "dialog-actions-group" but the actual class name is "myds-dialog-actions-group" to match the implementation.
| max-width: 40rem; /* Custom dialog width */ | ||
| } | ||
|
|
||
| .dialog-title { |
There was a problem hiding this comment.
The Customization example uses .dialog-title which doesn't match the actual CSS selector .myds-dialog-title. This example won't work as shown.
| .dialog-title { | |
| .myds-dialog-title { |
| .dark .dialog-close:hover { | ||
| background-color: #27272a; | ||
| border-color: #3f3f46; | ||
| color: #ffffff; | ||
| } | ||
|
|
||
| .dark .dialog-close:focus { | ||
| box-shadow: 0 0 0 3px #1d4ed866; | ||
| } | ||
|
|
||
| .dark .dialog-close:disabled { | ||
| background-color: #18181b; | ||
| color: #52525b; | ||
| border-color: transparent; | ||
| } | ||
|
|
||
| .dark .dialog-title { | ||
| color: #f9fafb; | ||
| } | ||
|
|
||
| /* Dialog Content */ | ||
| .dark .dialog-content { |
There was a problem hiding this comment.
The DialogDarkCSS tab documentation shows incorrect class names. The selectors .dialog-close, .dialog-title, and .dialog-content should be .myds-dialog-close, .myds-dialog-title, and .myds-dialog-content to match the actual CSS implementation in dialog-dark.css.
| .dark .dialog-close:hover { | |
| background-color: #27272a; | |
| border-color: #3f3f46; | |
| color: #ffffff; | |
| } | |
| .dark .dialog-close:focus { | |
| box-shadow: 0 0 0 3px #1d4ed866; | |
| } | |
| .dark .dialog-close:disabled { | |
| background-color: #18181b; | |
| color: #52525b; | |
| border-color: transparent; | |
| } | |
| .dark .dialog-title { | |
| color: #f9fafb; | |
| } | |
| /* Dialog Content */ | |
| .dark .dialog-content { | |
| .dark .myds-dialog-close:hover { | |
| background-color: #27272a; | |
| border-color: #3f3f46; | |
| color: #ffffff; | |
| } | |
| .dark .myds-dialog-close:focus { | |
| box-shadow: 0 0 0 3px #1d4ed866; | |
| } | |
| .dark .myds-dialog-close:disabled { | |
| background-color: #18181b; | |
| color: #52525b; | |
| border-color: transparent; | |
| } | |
| .dark .myds-dialog-title { | |
| color: #f9fafb; | |
| } | |
| /* Dialog Content */ | |
| .dark .myds-dialog-content { |
| padding: 1.5rem 1.5rem 0.5rem; | ||
| } | ||
|
|
||
| .dialog-title { |
There was a problem hiding this comment.
The class name should be .myds-dialog-title instead of .dialog-title to match the HTML markup in ta-dialog-preview.html which uses class="myds-dialog-title". This inconsistency will cause the title styling to not be applied.
| .dialog-title { | |
| .myds-dialog-title { |
| .dark .dialog-close:hover { | ||
| background-color: #27272a; | ||
| border-color: #3f3f46; | ||
| color: #ffffff; | ||
| } | ||
|
|
||
| .dark .dialog-close:focus { | ||
| box-shadow: 0 0 0 3px #1d4ed866; | ||
| } | ||
|
|
||
| .dark .dialog-close:disabled { | ||
| background-color: #18181b; | ||
| color: #52525b; | ||
| border-color: transparent; | ||
| } | ||
|
|
||
| .dark .dialog-title { | ||
| color: #f9fafb; | ||
| } | ||
|
|
||
| /* Dialog Content */ | ||
| .dark .dialog-content { |
There was a problem hiding this comment.
The DialogDarkCSS tab documentation shows incorrect class names. The selectors .dialog-close, .dialog-title, and .dialog-content should be .myds-dialog-close, .myds-dialog-title, and .myds-dialog-content to match the actual CSS implementation in dialog-dark.css.
| .dark .dialog-close:hover { | |
| background-color: #27272a; | |
| border-color: #3f3f46; | |
| color: #ffffff; | |
| } | |
| .dark .dialog-close:focus { | |
| box-shadow: 0 0 0 3px #1d4ed866; | |
| } | |
| .dark .dialog-close:disabled { | |
| background-color: #18181b; | |
| color: #52525b; | |
| border-color: transparent; | |
| } | |
| .dark .dialog-title { | |
| color: #f9fafb; | |
| } | |
| /* Dialog Content */ | |
| .dark .dialog-content { | |
| .dark .myds-dialog-close:hover { | |
| background-color: #27272a; | |
| border-color: #3f3f46; | |
| color: #ffffff; | |
| } | |
| .dark .myds-dialog-close:focus { | |
| box-shadow: 0 0 0 3px #1d4ed866; | |
| } | |
| .dark .myds-dialog-close:disabled { | |
| background-color: #18181b; | |
| color: #52525b; | |
| border-color: transparent; | |
| } | |
| .dark .myds-dialog-title { | |
| color: #f9fafb; | |
| } | |
| /* Dialog Content */ | |
| .dark .myds-dialog-content { |
| .myds-page-center { | ||
| display: grid; | ||
| place-content: center; | ||
| min-height: 100%; | ||
| } |
There was a problem hiding this comment.
Inconsistent naming convention with existing components. This file defines .myds-page-center, but the button component uses .page-center (see apps/docs/public/assets/ta-preview/button/button.css line 173). For consistency across the MYDS component library, consider using .page-center to match the existing pattern, or update the button component to use the myds- prefix.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 7 changed files in this pull request and generated 25 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button class="button-myds button-myds-secondary" type="button" onclick=closeDialog() autofocus> | ||
| Secondary Action | ||
| </button> | ||
| <button class="button-myds button-myds-primary" type="button" onclick=closeDialog()> |
There was a problem hiding this comment.
Missing quotes around the onclick attribute values. HTML attributes should be properly quoted. Change onclick=closeDialog() to onclick="closeDialog()".
| </div> | ||
| </div> | ||
|
|
||
| <button class="myds-dialog-close" onclick=closeDialog() aria-label="Close"> |
There was a problem hiding this comment.
Missing quotes around the onclick attribute value. HTML attributes should be properly quoted. Change onclick=closeDialog() to onclick="closeDialog()".
| <button class="button-myds button-myds-secondary" type="button" onclick=closeDialog()> | ||
| Secondary Action | ||
| </button> | ||
| <button class="button-myds button-myds-primary" type="button" onclick=closeDialog()> |
There was a problem hiding this comment.
Missing quotes around the onclick attribute values. HTML attributes should be properly quoted. Change onclick=closeDialog() to onclick="closeDialog()".
| </div> | ||
| </div> | ||
|
|
||
| <button class="myds-dialog-close" onclick=closeDialog() aria-label="Close"> |
There was a problem hiding this comment.
Missing quotes around the onclick attribute value. HTML attributes should be properly quoted. Change onclick=closeDialog() to onclick="closeDialog()".
| </div> | ||
| </div> | ||
|
|
||
| <button class="myds-dialog-close" onclick=closeDialog() aria-label="Close"> |
There was a problem hiding this comment.
Missing quotes around the onclick attribute value. HTML attributes should be properly quoted. Change onclick=closeDialog() to onclick="closeDialog()".
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| </head> | ||
| <body> | ||
| <main class="myds-page-center"> |
There was a problem hiding this comment.
Class naming inconsistency: Uses myds-page-center while the established convention in the codebase uses page-center (see apps/docs/public/assets/ta-preview/button/ta-button-preview.html:30). Consider changing to page-center for consistency across components.
| <main class="myds-page-center"> | |
| <main class="page-center"> |
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| </head> | ||
| <body> | ||
| <main class="myds-page-center"> |
There was a problem hiding this comment.
Class naming inconsistency: Uses myds-page-center while the established convention in the codebase uses page-center (see apps/docs/public/assets/ta-preview/button/ta-button-preview.html:30). Consider changing to page-center for consistency across components.
| <main class="myds-page-center"> | |
| <main class="page-center"> |
| outline: 3px solid #96b7ff66; | ||
| } | ||
|
|
||
| .myds-page-center { |
There was a problem hiding this comment.
Class naming inconsistency: Uses .myds-page-center in the documentation while the established convention in the codebase uses .page-center (see apps/docs/public/assets/ta-preview/button/button.css:173). Consider changing to .page-center for consistency across components.
| .myds-page-center { | |
| .page-center { |
| </div> | ||
| </div> | ||
|
|
||
| <button class="myds-dialog-close" onclick=closeDialog() aria-label="Close"> |
There was a problem hiding this comment.
Missing quotes around the onclick attribute value. HTML attributes should be properly quoted. Change onclick=closeDialog() to onclick="closeDialog()".
| </head> | ||
| <body> | ||
| <main class="myds-page-center"> | ||
| <button class="button-myds-primary" onclick=openDialog()> |
There was a problem hiding this comment.
Missing quotes around the onclick attribute value. HTML attributes should be properly quoted. Change onclick=openDialog() to onclick="openDialog()".
Summarise the feature
Issue ticket # SSD-805
Description:
Type of change
Checklist