Skip to content

Conversation

@dmontesi
Copy link

@dmontesi dmontesi commented Jan 7, 2026

  • New overlay component

@netlify
Copy link

netlify bot commented Jan 7, 2026

Deploy Preview for imarc-pronto ready!

Name Link
🔨 Latest commit d224609
🔍 Latest deploy log https://app.netlify.com/projects/imarc-pronto/deploys/69738012af37f80008a0ce8a
😎 Deploy Preview https://deploy-preview-20--imarc-pronto.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 project configuration.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new “overlay” molecule built on the native <dialog> element, and introduces a dialog molecule stylesheet/demo while updating SCSS forwards.

Changes:

  • Added overlay HTML demos and SCSS for a full-screen dialog-based overlay.
  • Added a dialog molecule (SCSS + HTML demo) and forwarded it via molecules/index.scss.
  • Removed the dialog forward from organisms/index.scss and added forwards for dialog and overlay under molecules.

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
resources/styles/organisms/index.scss Removes forwarding of dialog from organisms.
resources/styles/molecules/index.scss Forwards new dialog and overlay molecule styles.
resources/styles/molecules/overlay/index.scss Implements overlay styling (full-screen dialog + backdrop behavior).
resources/styles/molecules/overlay/overlay.html Adds a closed-state overlay demo with an open trigger.
resources/styles/molecules/overlay/overlay--opened.html Adds an opened-state overlay demo (static open attribute + simulated backdrop).
resources/styles/molecules/dialog/index.scss Adds dialog styling and close-button positioning rules.
resources/styles/molecules/dialog/dialog.html Adds a dialog markup demo with a close button.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 6 to 7
<button class="button" onclick="document.querySelector('.overlay').showModal()">Open Overlay</button>
</div>
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

These HTML component demos appear to avoid inline event handlers (e.g., resources/styles/molecules/accordion/accordion.html:1-30 and resources/styles/molecules/mobileNavigation/mobileNavigation.html:22-25 use markup/framework bindings, not onclick). Using inline onclick here makes the snippet inconsistent and harder to reuse in templated contexts. Consider switching to the project’s usual event binding approach (e.g., framework @click) or documenting the required JS separately.

Copilot uses AI. Check for mistakes.
@khamer
Copy link
Member

khamer commented Jan 30, 2026

Is the idea for overlay--opened.html to be an example of something like intro/splash? My main feedback on that part is I think I'd rather see that be implemented as a modifier, like .overlay.-splash or .overlay.-whatever, that makes it more clear that the base version is for like, a modal that the user triggers and the second is a modal that a user is forced to use and can't be reopened. \

These two use cases might be separate enough to even merit two different components rather than the sibling .overlay__backdrop. I'm also not wild about the &:not(:has(+ .overlay[open])) { selector, I understand what you're doing and we can live with it, but if they were separate components it might be simpler? I'm open to ideas.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants