Skip to content

Site dialog refactor - #355

Draft
gauthierm wants to merge 7 commits into
silverorange:masterfrom
gauthierm:site-dialog-refactor
Draft

Site dialog refactor#355
gauthierm wants to merge 7 commits into
silverorange:masterfrom
gauthierm:site-dialog-refactor

Conversation

@gauthierm

Copy link
Copy Markdown
Member

Description

Draft to move site-dialog to DOM native APIs instead of using YUI2 utils. Ideally this entire file could be replaced with a standard <dialog> element. The current work in progress strips out the deprecated YUI2 APIs.

Testing Instructions (optional)

Add step-by-step instructions for testing the PR, if necessary.

  1. Check out this PR

Developer Checklist

Before requesting review for this PR, make sure the following tasks are
complete:

  • I added a link to the relevant Shortcut story, if applicable
  • I added testing instructions, if any
  • I made sure existing CI checks pass
  • I checked that all requirements of the ticket are fulfilled

Reviewer Checklist

Before merging this PR, make sure the following tasks are complete:

  • I made sure there are no active labels that block merge
  • I followed the testing instructions
  • I made sure the CI checks pass
  • I reviewed the file changes on GitHub
  • I checked that all requirements of the ticket (if any) are fulfilled

@gauthierm
gauthierm marked this pull request as draft August 10, 2026 18:03
parseInt(Dom.getStyle(this.container, 'marginTop')) +
parseInt(Dom.getStyle(this.container, 'marginBottom'));
let margin =
Math.parseInt(container_style.marginTop) +

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Number, not Math

parseInt(Dom.getStyle(this.container, 'marginTop')) +
parseInt(Dom.getStyle(this.container, 'marginBottom'));
let margin =
Math.parseInt(container_style.marginTop) +

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Number, not Math

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.

1 participant