feat: responsive SWAL#2222
Conversation
WalkthroughThe changes update the SweetAlert JavaScript integration with a refreshed, functionally equivalent version of the library and extensively refactor its CSS. The Changes
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
emhttp/plugins/dynamix/javascript/dynamix.js (1)
51-51: jQuery Cookie plugin update looks good, but consider modern alternativesThe jQuery Cookie plugin has been properly updated with SameSite support defaulting to 'Lax', which is good for security. However, the minified code makes detailed review difficult.
Consider these modern alternatives:
- Use the native
document.cookieAPI with proper SameSite handling- Migrate to a more modern cookie library that doesn't depend on jQuery
- Use the Web Storage API (localStorage/sessionStorage) where appropriate
// Modern cookie handling example without jQuery dependency function setCookie(name, value, options = {}) { const defaults = { path: '/', sameSite: 'Lax' }; const config = { ...defaults, ...options }; let cookieString = `${encodeURIComponent(name)}=${encodeURIComponent(value)}`; if (config.expires) { cookieString += `; expires=${config.expires.toUTCString()}`; } if (config.path) { cookieString += `; path=${config.path}`; } if (config.sameSite) { cookieString += `; sameSite=${config.sameSite}`; } document.cookie = cookieString; }emhttp/plugins/dynamix/styles/jquery.sweetalert.css (2)
27-33: Add fallback generic fonts
Usingfont-family: clear-sans;without a generic family can lead to unexpected fallbacks. It’s best practice to include a fallback:- font-family: clear-sans; + font-family: clear-sans, sans-serif;
459-483: Remove duplicate typography rules
You define.sweet-alert h2andpstyles twice—once via nesting (which is invalid) and again as flat selectors. After flattening the nested rules, remove one of these blocks to avoid redundancy and potential style conflicts.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
emhttp/plugins/dynamix/javascript/dynamix.js(1 hunks)emhttp/plugins/dynamix/sheets/ArrayOperation.css(1 hunks)emhttp/plugins/dynamix/styles/default-base.css(10 hunks)emhttp/plugins/dynamix/styles/jquery.sweetalert.css(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
emhttp/plugins/dynamix/styles/jquery.sweetalert.css
[error] 94-94: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 100-100: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 501-501: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
[error] 508-508: Unknown property is not allowed.
See CSS Specifications and browser specific properties for more details.
To resolve this issue, replace the unknown property with a valid CSS property.
(lint/correctness/noUnknownProperty)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (11)
emhttp/plugins/dynamix/sheets/ArrayOperation.css (1)
48-55: Approve: Exclude SweetAlert buttons from responsive table margin reset
Removing.sweet-alert buttonensures SweetAlert dialogs keep their own styling without being affected by the table’s button margin rules.emhttp/plugins/dynamix/styles/default-base.css (10)
172-178: Approve: Exclude SweetAlert buttons from base button typography styles
Removing.sweet-alert buttonprevents SweetAlert buttons from inheriting the general button font settings, keeping their dedicated styles intact.
210-217: Approve: Remove SweetAlert buttons from no-margin button group
Excluding.sweet-alert buttonfrom the.buttons-no-marginrules avoids unintended margin resets on alert buttons.
231-237: Approve: Add focus styling fortextareaelements
Includingtextarea:focusalongside input focus ensures consistent focus visuals across form controls.
241-246: Approve: Extend hover effect toa.buttonlinks
Addinga.button:hoveraligns link-styled buttons with the same hover feedback as native buttons.
257-267: Approve: Includea.button:active[disabled]in disabled state
Ensures disabled link-buttons mirror the opacity and cursor behavior of disabled inputs and buttons.
2085-2091: Approve: Exclude SweetAlert buttons from sidebar base styles
Removing.sweet-alert buttonhere keeps the sidebar’s button styles scoped to its own controls.
2102-2108: Approve: Addtextarea:focusoverride in sidebar theme
This addition ensures text areas in the sidebar receive the same focus treatment as inputs.
2114-2119: Approve: Includea.button:hoverin sidebar hover styles
Consistent hover states for link-styled buttons enhance UI coherence in the sidebar.
2126-2131: Approve: Adda.button:activein sidebar active state
Provides visual feedback for active link-buttons within the sidebar layout.
2136-2148: Approve: Includetextarea[disabled]in sidebar disabled styles
Ensures disabled text areas in the sidebar adopt the intended disabled appearance.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
emhttp/plugins/dynamix/styles/jquery.sweetalert.css (2)
27-407: Fix SASS-style nesting: nested rules invalid in plain CSS
The entire.sweet-alertblock (lines 27–407) uses nested selectors (e.g.,h2 {…},&.show {…},&::before, etc.), which are not supported in vanilla CSS. Browsers will ignore these nested rules, causing many styles to be lost. You should either:
- Convert this file to a CSS preprocessor format (e.g., rename to
.scssand include in your build), or- Flatten all nested selectors into valid CSS, for example:
.sweet-alert h2 { … } .sweet-alert .sa-error-container.show { … } .sweet-alert .sa-input-error::before { … }
409-456: Fix nested selectors in.sweet-alert.nchanblock
Within the.sweet-alert.nchanrules (lines 410–456), nested blocks likeh2 {…},pre { h2 {…} p {…} }, andp {…}are invalid in plain CSS. Flatten these to valid selectors, for example:.sweet-alert.nchan h2 { … } .sweet-alert.nchan pre h2 { … } .sweet-alert.nchan pre p { … }
🧹 Nitpick comments (7)
emhttp/plugins/dynamix/javascript/dynamix.js (1)
50-51: Normalize cookieSameSiteattribute casing.The jQuery Cookie plugin emits
; samesite=…, but the standard attribute name isSameSite. For clarity and consistency, update it as follows:- "; samesite="+o.samesite + "; SameSite="+o.samesiteOptionally, consider defaulting
HttpOnlyif the cookies are meant to be inaccessible from JavaScript.emhttp/plugins/dynamix/styles/jquery.sweetalert.css (6)
458-484: Remove duplicate typography definitions
Heading and paragraph styles are declared both nested inside.sweet-alert(lines 49–60, 62–73) and again flattened (lines 458–484). After you flatten nested rules, remove one of these duplicate blocks to avoid redundancy.
85-102: Consolidate.sa-error-containerstyles
The.sa-error-containerblock is defined nested (lines 85–102) and then again in flat form later (lines 492–510). Flatten your nested rules into the flat definitions and remove the duplicated section.
121-164: Consolidate.sa-input-errorstyles
.sa-input-errorappears as a nested block (lines 121–164) and also as flat selectors (lines 528–572). Keep only the flat definitions once you’ve flattened all rules.
166-192: Consolidate input field styles
Input-related selectors (input[type=text], focus, placeholder) are defined both nested (lines 166–192) and flat (lines 573–604). After flattening, remove the nested duplicates to streamline the file.
199-202: Consolidate.show-inputstyles
The.show-inputrules are declared nested (lines 199–202) and again flattened (lines 606–608). Retain only the flat definition post-flattening.
203-219: Consolidate loading spinner rules
The.la-ball-fallspinner is defined nested within.sweet-alert(lines 203–219) and again flat under.sweet-alert .la-ball-fall(lines 642–656). Flatten nested rules into the flat block and remove the duplicate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
emhttp/plugins/dynamix/javascript/dynamix.js(1 hunks)emhttp/plugins/dynamix/sheets/ArrayOperation.css(1 hunks)emhttp/plugins/dynamix/styles/default-base.css(10 hunks)emhttp/plugins/dynamix/styles/jquery.sweetalert.css(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
emhttp/plugins/dynamix/sheets/ArrayOperation.css (1)
48-55: Removed.sweet-alert buttonselector from responsive margin reset
Decouples SweetAlert-specific button styling from the generic responsive table layout. Ensure margin rules for SweetAlert buttons have been migrated to its dedicated stylesheet.emhttp/plugins/dynamix/styles/default-base.css (10)
172-177: Removed.sweet-alert buttonfrom global control styling
This extraction focuses generic button styles on standard controls and centralizes SweetAlert button styles in its own CSS.
210-215: Excluded.sweet-alert buttonfrom.buttons-no-margin, .buttons-spacedgroup
Good separation of concerns. Confirm SweetAlert button margin behavior is managed by its dedicated CSS.
236-242: Addedtextarea:focusto the focus state group
Ensures textareas receive the same focus background and outline behavior as text inputs.
241-246: Removed SweetAlert from hover state selectors
Decouples SweetAlert hover styles; verify that its dedicated stylesheet provides the expected hover effects.
270-272: Removed SweetAlert from active/disabled state selectors
Centralizes active and disabled state styling for SweetAlert buttons in its own CSS.
2085-2090: Removed.sweet-alert buttonfrom sidebar button group
Consistent with the global removal; please confirm that the SweetAlert CSS covers sidebar modal buttons as well.
2102-2108: Includedtextarea:focusin sidebar focus styling
Maintains consistent focus treatments for all form controls in the sidebar.
2114-2119: Removed SweetAlert from sidebar hover selectors
Ensure the SweetAlert stylesheet provides hover feedback within sidebar-themed dialogs.
2126-2131: Removed SweetAlert from sidebar active selectors
Active state styling should now be handled exclusively by SweetAlert's own CSS.
2136-2148: Removed SweetAlert from sidebar disabled selectors and addedtextarea[disabled]
Decouples default disabled styling and extends it to textareas. Verify that disabled SweetAlert buttons still render correctly.emhttp/plugins/dynamix/javascript/dynamix.js (1)
47-49:Details
❓ Verification inconclusive
Verify embedded SweetAlert version and audit for vulnerabilities.
You’ve refreshed the embedded SweetAlert block (still labeled v1.0.0) but it remains minified, which prevents security scanners and static analysis from flagging known issues. Please confirm:
- That this code exactly matches the intended official release (v1.0.0) and no unintended mix of versions occurred.
- Whether any patches from later v1.x releases (e.g., v1.1.3+) are missing.
- That regression testing covers all API paths (string vs. object arguments, AMD/CommonJS exports, input handling, etc.).
You can run a quick check against the npm registry and known advisories:
🏁 Script executed:
#!/bin/bash # Verify SweetAlert 1.x versions and check for advisories echo "Available SweetAlert v1.x versions:" curl -s https://registry.npmjs.org/sweetalert | jq '.versions | keys[]' | grep '"1\.' | sort -V echo "Checking npm audit for SweetAlert:" npm audit --package-lock-only --json \ | jq '.advisories | to_entries[] | select(.value.module_name=="sweetalert")'Length of output: 833
Action Required: Diff embedded SweetAlert vs. official v1.0.0/1.1.3 and audit
- Available v1.x releases on npm:
1.0.0, 1.0.0-beta, 1.0.1, 1.1.0, 1.1.1, 1.1.2, 1.1.3npm auditcouldn’t run without a lockfile, so no advisories surfaced.Please verify the following:
• Confirm the block in
emhttp/plugins/dynamix/javascript/dynamix.js(after the Bergware header) exactly matches the officialdist/sweetalert.min.jsfrom sweetalert@1.0.0.
• Extract and diff the same file from sweetalert@1.1.3—ensure you’re not missing any security patches in later v1.x releases.
• Add apackage-lock.json, rerunnpm audit --json | jq '.advisories'to catch any known issues.
• Validate regression tests cover all SweetAlert API paths:
– String vs. object invocation
– AMD/CommonJS exports
– Input/prompt handling, loader behavior, escape-key and outside-click options
3 participants
Summary by CodeRabbit