Open
Conversation
* Design system First draft * Feedback changes * feddback changes in css * feedback changes * required images * make scss & local fonts * input text css * removed the dev api and enter random key
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a new admin Connect page for Google API key configuration with activation redirect, AJAX-based validation flow, design system assets, and supporting SCSS stylesheets. Includes build script updates for SCSS compilation and UI overlay masking for incomplete setup states. Changes
Sequence DiagramssequenceDiagram
participant Admin as Admin User
participant Connect as Connect Page
participant JS as JavaScript
participant AJAX as AJAX Endpoint
participant API as Google API
Admin->>Connect: Visits Connect page
Connect->>JS: Initializes validation UI
Admin->>JS: Enters API key
JS->>JS: Validates input
Admin->>JS: Clicks validate button
JS->>AJAX: POST validate_google_api_key (key, nonce)
AJAX->>AJAX: Verify nonce & permission
AJAX->>API: GET /calendar/v3/calendars/{id}
API-->>AJAX: Response (200, 401, error)
alt API Success
AJAX-->>JS: {success: true}
JS->>JS: Update UI success state
JS->>JS: Animate progress circle
JS->>JS: Mark step completed
else API Key Not Supported
AJAX-->>JS: {error: true, reason: api_keys_not_supported}
JS->>JS: Show error message
else API Error
AJAX-->>JS: {error: true, message: error_text}
JS->>JS: Display error feedback
end
JS->>Connect: Render final state
sequenceDiagram
participant Plugin as Plugin Activation
participant Install as Installation Hook
participant Options as WP Options
participant Admin as Admin Dashboard
participant Redirect as Redirect Handler
Plugin->>Install: Activate plugin
Install->>Options: Set simple_calendar_redirect_to_connect = 1
Admin->>Redirect: Load admin interface
Redirect->>Redirect: Check user capability
Redirect->>Options: Read redirect flag
alt Flag set & not AJAX & not bulk
Redirect->>Options: Delete redirect flag
Redirect->>Admin: wp_safe_redirect to Connect page
else
Redirect->>Admin: Continue normal flow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
assets/scss/design-system.scss (1)
602-624: Keyframe names use snake_case instead of kebab-case.Stylelint flagged
sc_btn_loading_spinandsc_btn_loading_scaleas not following kebab-case convention. However, this appears intentional to maintain consistency with thesc_*naming pattern used throughout the design system. Consider adding a Stylelint exception for this file or these specific keyframes if the naming convention is deliberate.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/scss/design-system.scss` around lines 602 - 624, Stylelint is flagging keyframe names sc_btn_loading_spin and sc_btn_loading_scale for using snake_case instead of the project kebab-case convention; to fix, either rename these `@keyframes` to kebab-case (e.g., sc-btn-loading-spin, sc-btn-loading-scale) and update any references, or add a Stylelint exception for these selectors in the design-system.scss (or for this file) so the sc_* pattern is allowed; locate the `@keyframes` definitions (sc_btn_loading_spin and sc_btn_loading_scale) and apply the chosen approach consistently across their usages.assets/css/admin-post-settings.scss (1)
24-34: Verify Poppins font availability in admin context.The
.simcal-settings-mask__messageclass referencesfont-family: Poppins, sans-serif, but this file doesn't import the Poppins font. The design system (design-system.scss) defines it as'SC Poppins'via@font-face.Consider using the design system's font variable or ensure Poppins is loaded on the calendar post type edit screen.
♻️ Suggested fix
.simcal-settings-mask__message { - font-family: Poppins, sans-serif; + font-family: 'SC Poppins', Poppins, sans-serif; font-weight: 600;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/css/admin-post-settings.scss` around lines 24 - 34, The .simcal-settings-mask__message rule references Poppins but the SC Poppins `@font-face` defined in design-system.scss isn't imported here; either switch the font-family to use the design system's exported font (e.g., the SC Poppins variable or the 'SC Poppins' name defined in design-system.scss) or import design-system.scss into assets/css/admin-post-settings.scss and ensure that design-system styles are enqueued for the calendar post type edit screen in the admin. Update the .simcal-settings-mask__message declaration to use the design-system font identifier and confirm the admin stylesheet loads the design-system.scss so the font-face is available in the admin context.assets/design-system.html (1)
507-586: Consider removing or enabling the commented-out JavaScript.The commented-out JavaScript block contains interactive functionality for password toggle and loading button demos. If this showcase page is intended for active use, consider enabling the code. Otherwise, if it's not needed, removing it would reduce file size and avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/design-system.html` around lines 507 - 586, The commented-out script block containing initSquareToggle and initLoadingButtons should either be removed or re-enabled; locate the IIFE that defines initSquareToggle, initLoadingButtons and the related selectors (sc_icon--square, sc_input_password_toggle, data-sc-loading-btn) and either 1) uncomment the entire immediately-invoked function so the password toggle and loading-button demos work, or 2) delete the whole commented block to reduce file size and avoid confusion—make sure to keep any necessary ARIA/title logic inside initSquareToggle if you re-enable it.assets/js/admin.js (1)
707-710: Consider reducing the auto-submit delay.The 10-second delay before auto-submitting the form after successful validation may feel sluggish to users. A shorter delay (e.g., 1.5-2 seconds) would provide enough time for the success animation while improving perceived responsiveness.
♻️ Proposed change
$form.data('scValidated', true); submitTimer = setTimeout(function () { if ($btn.length) $btn.prop('disabled', false); $form.trigger('submit'); - }, 10000); + }, 1500);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/js/admin.js` around lines 707 - 710, The current auto-submit uses a 10000ms timeout which feels sluggish; change the timeout passed to setTimeout in the submitTimer block (the code creating submitTimer that re-enables $btn and triggers $form.submit()) to a shorter value such as 1500–2000ms so the success animation can complete but the form submits faster; locate the setTimeout call that references submitTimer, $btn, and $form and replace 10000 with the chosen lower millisecond value.assets/scss/connect.scss (1)
441-441: Use design system token instead of hardcoded hex color.The color
#1d73beis hardcoded here while other styles consistently use design system tokens likevar(--sc-color-blue). This inconsistency could cause maintenance issues if the color palette changes.♻️ Proposed fix
.sc_connect_pro_list_icon { width: 20px; height: 20px; border-radius: 50%; - background: `#1d73be`; + background: var(--sc-color-blue); display: inline-flex; align-items: center; justify-content: center; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/scss/connect.scss` at line 441, Replace the hardcoded background color "#1d73be" with the design system token (for example use var(--sc-color-blue)) in the CSS rule where background: `#1d73be`; is set so the style uses the shared token; ensure you remove the hex and use the token consistently and add a fallback if needed (e.g., background: var(--sc-color-blue, `#1d73be`)).includes/functions/admin.php (1)
386-393: Potential race condition in completion timestamp tracking.If two requests occur simultaneously when
$has_published_calendarbecomes true, both could callupdate_option(). While not critical (the timestamp would just be slightly different), consider usingadd_option()with autoload or checking if the option already has a non-zero value before updating.♻️ Proposed fix
// Track when setup first reached 100% so we can hide the progress card after a day. $completed_timestamp = (int) get_option('simple_calendar_connect_setup_completed_at', 0); - if ($has_published_calendar && $completed_timestamp <= 0) { + if ($has_published_calendar && $completed_timestamp === 0) { $completed_timestamp = time(); - update_option('simple_calendar_connect_setup_completed_at', $completed_timestamp); + // Only set if not already set (avoid race condition) + add_option('simple_calendar_connect_setup_completed_at', $completed_timestamp, '', 'no'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@includes/functions/admin.php` around lines 386 - 393, When setting the completion time for simple_calendar_connect_setup_completed_at, avoid blindly calling update_option() on concurrent requests: first read the current value with get_option('simple_calendar_connect_setup_completed_at', 0) into $completed_timestamp and only set it if it's <= 0; better yet try add_option('simple_calendar_connect_setup_completed_at', $completed_timestamp, '', 'no') (or equivalent add_option call) when $has_published_calendar is true and only fall back to update_option() if add_option indicated the option did not already exist—this ensures the first writer wins and prevents race updates from stomping each other while preserving the existing logic around $completed_timestamp and $hide_progress_after.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.bin/esbuild.mjs:
- Line 3: The import line "import pkg from '../package.json' with { type: 'json'
}" uses the newer "with" JSON-import syntax unsupported by Node.js >=18.15.0 and
<18.20.0; fix by either raising the declared minimum Node version in
package.json to ">=18.20.0" (or ">=18.20.5") or revert the import to the
compatible assert syntax ("import pkg from '../package.json' assert { type:
'json' }") so the pkg import works across the declared Node range.
In `@assets/js/admin.js`:
- Around line 696-732: The `api_keys_not_supported` branch treats the response
as success but omits showing the Add Calendar button; update the else-if branch
handling `res.data.reason === 'api_keys_not_supported'` to mirror the success
branch by calling animateProgressToApiKeyCompleted(), showing $msgWrap and
$msgSuccess, adding the sc_input--success class to $wrap, and also calling
$('#sc_connect_add_calendar_btn').show(); ensure the same $btn state changes,
$form.data('scValidated', true) and the submitTimer behavior are preserved so
the UI feedback matches the success path (see animateProgressToApiKeyCompleted,
$wrap, $msgWrap, $msgSuccess, $btn, $form, submitTimer).
In `@assets/scss/design-system.scss`:
- Around line 1376-1384: The .sc_ds_setup_card rule uses an undefined CSS
variable --sc-spacing-40 for gap; either add a new spacing token named
--sc-spacing-40 in the spacing variables block (next to
--sc-spacing-0..--sc-spacing-25) with the desired size, or change the gap to an
existing defined token (e.g., replace --sc-spacing-40 with an appropriate
existing variable such as --sc-spacing-25 or another defined spacing variable);
update any other references if you add the new token so spacing variables remain
consistent.
In `@includes/admin/menus.php`:
- Around line 142-145: The $video_url variable created via
apply_filters('simple_calendar_connect_welcome_video_url',
'https://www.youtube.com/embed/VIDEO_ID') is unused; either remove that
assignment entirely or replace the static placeholder image output with an
embedded video using $video_url (for example render an iframe whose src is
$video_url). Update the code around the placeholder image rendering in the same
template so the variable is actually used if you choose the embed approach, or
simply delete the $video_url line if no video is required.
In `@includes/functions/admin.php`:
- Line 394: Remove the unused local nonce variable by deleting the line that
assigns $validate_nonce =
wp_create_nonce('simcal_connect_validate_google_api_key') in
includes/functions/admin.php; ensure no other code in that file references
$validate_nonce and rely on the existing AJAX nonce provided by
wp_localize_script in includes/admin/assets.php (the localized key created
around the AJAX validation code).
In `@package.json`:
- Line 40: The npm script build:admin-ui-css currently passes multiple file
paths without source:destination pairs to the Sass CLI; update the script so
each compilation is a source:destination pair by changing the two entries to use
colons (e.g. design-system.scss:assets/generated/design-system.css and
connect.scss:assets/generated/connect.css) so Sass receives proper
source:destination arguments for both design-system and connect builds.
---
Nitpick comments:
In `@assets/css/admin-post-settings.scss`:
- Around line 24-34: The .simcal-settings-mask__message rule references Poppins
but the SC Poppins `@font-face` defined in design-system.scss isn't imported here;
either switch the font-family to use the design system's exported font (e.g.,
the SC Poppins variable or the 'SC Poppins' name defined in design-system.scss)
or import design-system.scss into assets/css/admin-post-settings.scss and ensure
that design-system styles are enqueued for the calendar post type edit screen in
the admin. Update the .simcal-settings-mask__message declaration to use the
design-system font identifier and confirm the admin stylesheet loads the
design-system.scss so the font-face is available in the admin context.
In `@assets/design-system.html`:
- Around line 507-586: The commented-out script block containing
initSquareToggle and initLoadingButtons should either be removed or re-enabled;
locate the IIFE that defines initSquareToggle, initLoadingButtons and the
related selectors (sc_icon--square, sc_input_password_toggle,
data-sc-loading-btn) and either 1) uncomment the entire immediately-invoked
function so the password toggle and loading-button demos work, or 2) delete the
whole commented block to reduce file size and avoid confusion—make sure to keep
any necessary ARIA/title logic inside initSquareToggle if you re-enable it.
In `@assets/js/admin.js`:
- Around line 707-710: The current auto-submit uses a 10000ms timeout which
feels sluggish; change the timeout passed to setTimeout in the submitTimer block
(the code creating submitTimer that re-enables $btn and triggers $form.submit())
to a shorter value such as 1500–2000ms so the success animation can complete but
the form submits faster; locate the setTimeout call that references submitTimer,
$btn, and $form and replace 10000 with the chosen lower millisecond value.
In `@assets/scss/connect.scss`:
- Line 441: Replace the hardcoded background color "#1d73be" with the design
system token (for example use var(--sc-color-blue)) in the CSS rule where
background: `#1d73be`; is set so the style uses the shared token; ensure you
remove the hex and use the token consistently and add a fallback if needed
(e.g., background: var(--sc-color-blue, `#1d73be`)).
In `@assets/scss/design-system.scss`:
- Around line 602-624: Stylelint is flagging keyframe names sc_btn_loading_spin
and sc_btn_loading_scale for using snake_case instead of the project kebab-case
convention; to fix, either rename these `@keyframes` to kebab-case (e.g.,
sc-btn-loading-spin, sc-btn-loading-scale) and update any references, or add a
Stylelint exception for these selectors in the design-system.scss (or for this
file) so the sc_* pattern is allowed; locate the `@keyframes` definitions
(sc_btn_loading_spin and sc_btn_loading_scale) and apply the chosen approach
consistently across their usages.
In `@includes/functions/admin.php`:
- Around line 386-393: When setting the completion time for
simple_calendar_connect_setup_completed_at, avoid blindly calling
update_option() on concurrent requests: first read the current value with
get_option('simple_calendar_connect_setup_completed_at', 0) into
$completed_timestamp and only set it if it's <= 0; better yet try
add_option('simple_calendar_connect_setup_completed_at', $completed_timestamp,
'', 'no') (or equivalent add_option call) when $has_published_calendar is true
and only fall back to update_option() if add_option indicated the option did not
already exist—this ensures the first writer wins and prevents race updates from
stomping each other while preserving the existing logic around
$completed_timestamp and $hide_progress_after.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 51b70c3f-7a81-408a-8c84-269f84d32c4f
⛔ Files ignored due to path filters (23)
assets/fonts/Inter-Variable-Italic.ttfis excluded by!**/*.ttfassets/fonts/Inter-Variable.ttfis excluded by!**/*.ttfassets/images/pages/connect/check.svgis excluded by!**/*.svgassets/images/pages/connect/checked.svgis excluded by!**/*.svgassets/images/pages/connect/clapperboard.svgis excluded by!**/*.svgassets/images/pages/connect/close.svgis excluded by!**/*.svgassets/images/pages/connect/copy-white.svgis excluded by!**/*.svgassets/images/pages/connect/copy.svgis excluded by!**/*.svgassets/images/pages/connect/crown.svgis excluded by!**/*.svgassets/images/pages/connect/document.svgis excluded by!**/*.svgassets/images/pages/connect/eye-hide-white.svgis excluded by!**/*.svgassets/images/pages/connect/eye-hide.svgis excluded by!**/*.svgassets/images/pages/connect/eye-white.svgis excluded by!**/*.svgassets/images/pages/connect/eye.svgis excluded by!**/*.svgassets/images/pages/connect/headphone.svgis excluded by!**/*.svgassets/images/pages/connect/loading.svgis excluded by!**/*.svgassets/images/pages/connect/logo.pngis excluded by!**/*.pngassets/images/pages/connect/question-white-small.svgis excluded by!**/*.svgassets/images/pages/connect/question-white.svgis excluded by!**/*.svgassets/images/pages/connect/question.svgis excluded by!**/*.svgassets/images/pages/connect/star.svgis excluded by!**/*.svgassets/images/pages/connect/warning.svgis excluded by!**/*.svgassets/images/pages/connect/welcome-video-placeholder.pngis excluded by!**/*.png
📒 Files selected for processing (15)
.bin/esbuild.mjsassets/css/admin-post-settings.scssassets/design-system.htmlassets/js/admin.jsassets/scss/connect.scssassets/scss/design-system.scsscomposer.jsonincludes/admin/ajax.phpincludes/admin/assets.phpincludes/admin/menus.phpincludes/admin/metaboxes/settings.phpincludes/functions/admin.phpincludes/installation.phpincludes/main.phppackage.json
rosinghal
reviewed
Mar 16, 2026
rosinghal
reviewed
Mar 17, 2026
includes/functions/admin.php
Outdated
| ); ?> | ||
| </p> | ||
|
|
||
| <a href="https://simplecalendar.io/addons/google-calendar-pro/" target="_blank" class="sc_connect_pro_link"> |
Contributor
Author
There was a problem hiding this comment.
added in backlog will add for all the places in last.
rosinghal
reviewed
Mar 20, 2026
| aria-controls="sc_google_api_key" | ||
| title="<?php esc_attr_e('Show API key', 'google-calendar-events'); ?>" | ||
| > | ||
| <img src="<?php echo esc_url($assets_base . 'eye.svg'); ?>" alt="" class="sc_input_square_show" hidden /> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores