-
Notifications
You must be signed in to change notification settings - Fork 103
update that says everyone has to update godot versions #707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ChanminK
wants to merge
5
commits into
hackclub:main
Choose a base branch
from
ChanminK:main
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+188
−0
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
193d42c
made the update for hackatime
ChanminK f536e5d
Merge branch 'hackclub:main' into main
ChanminK 0ccec7b
getting rid of site-wide noti
ChanminK 806d279
Merge branch 'main' of https://github.com/ChanminK/hackatime
ChanminK b1e9ce4
Merge branch 'hackclub:main' into main
ChanminK File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
31 changes: 31 additions & 0 deletions
31
app/javascript/controllers/godot_wakatime_upgrade_warning_controller.js
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import { Controller } from "@hotwired/stimulus" | ||
|
|
||
| export default class extends Controller { | ||
| static targets = ["modal"] | ||
|
|
||
| connect() { | ||
| // Handle trailing slashes or query params safely | ||
| if (!window.location.pathname.startsWith("/docs/editors/godot")) return | ||
|
|
||
| const key = this.storageKey() | ||
| const dismissed = window.localStorage.getItem(key) | ||
|
|
||
| if (!dismissed) this.open() | ||
| } | ||
|
|
||
| open() { | ||
| this.modalTarget.classList.remove("hidden") | ||
| document.documentElement.classList.add("overflow-hidden") | ||
| } | ||
|
|
||
| dismiss() { | ||
| window.localStorage.setItem(this.storageKey(), "1") | ||
| this.modalTarget.classList.add("hidden") | ||
| document.documentElement.classList.remove("overflow-hidden") | ||
| } | ||
|
|
||
| storageKey() { | ||
| // bump this if you ever need to force-show again | ||
| return "hackatime_godot_wakatime_upgrade_warning_v1_dismissed" | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import { Controller } from "@hotwired/stimulus" | ||
|
|
||
| export default class extends Controller { | ||
| static targets = ["modal", "banner"] | ||
|
|
||
| connect() { | ||
| // Banner in case we ever want to use it | ||
| // if (this.hasBannerTarget) { | ||
| // this.bannerTarget.classList.remove("hidden") | ||
| // } | ||
|
|
||
| // side-wide noti scooter use it if ya want | ||
| // if (!this.modalDismissed()) { | ||
| // this.openModal() | ||
| // } | ||
| } | ||
|
|
||
| openModal() { | ||
| this.modalTarget.classList.remove("hidden") | ||
| document.documentElement.classList.add("overflow-hidden") | ||
| } | ||
|
|
||
| dismissModal() { | ||
| window.localStorage.setItem(this.modalKey(), "1") | ||
| this.modalTarget.classList.add("hidden") | ||
| document.documentElement.classList.remove("overflow-hidden") | ||
| } | ||
|
|
||
| // Banner dismissa; | ||
| dismissBanner() { | ||
| this.bannerTarget.classList.add("hidden") | ||
| } | ||
|
|
||
| modalDismissed() { | ||
| return window.localStorage.getItem(this.modalKey()) === "1" | ||
| } | ||
|
|
||
| modalKey() { | ||
| return "hackatime_sitewide_required_update_modal_v1" | ||
| } | ||
| } | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -251,5 +251,6 @@ | |
| method: "delete" | ||
| } | ||
| ] %> | ||
| <%= render "shared/sitewide_notice" %> | ||
| </body> | ||
| </html> | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| <div data-controller="sitewide-notice" class="not-prose"> | ||
| <!-- Top banner (dismissible, can persist even after modal is dismissed) --> | ||
| <div | ||
| data-sitewide-notice-target="banner" | ||
| class="hidden fixed top-0 left-0 right-0 z-[9998]" | ||
| > | ||
| <div class="mx-auto max-w-6xl px-4 pt-4"> | ||
| <div class="rounded-2xl border border-red-500/40 bg-red-500/10 px-4 py-3 shadow-lg"> | ||
| <div class="flex items-start justify-between gap-4"> | ||
| <div class="text-sm text-zinc-100"> | ||
| <span class="font-semibold uppercase tracking-wide text-red-200">Required update (mandatory):</span> | ||
| All users must install the newest version of <span class="font-semibold">Godot Super WakaTime</span>: | ||
| <a class="underline break-all" href="https://github.com/BudzioT/Godot_Super-Wakatime" target="_blank" rel="noreferrer"> | ||
| https://github.com/BudzioT/Godot_Super-Wakatime | ||
| </a> | ||
| <span class="block mt-1"> | ||
| Effective <span class="font-semibold">January 1, 2026</span>, any Hackatime project using the old Godot time-tracking plugin/extension will be | ||
| <span class="font-semibold">immediately rejected</span>. | ||
| </span> | ||
| </div> | ||
|
|
||
| <button | ||
| type="button" | ||
| class="shrink-0 rounded-lg bg-zinc-900/60 px-3 py-2 text-xs font-semibold text-white hover:bg-zinc-900" | ||
| data-action="sitewide-notice#dismissBanner" | ||
| aria-label="Dismiss notice banner" | ||
| > | ||
| Dismiss | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <!-- Modal popup (one-time, centered, not clipped) --> | ||
| <div | ||
| data-sitewide-notice-target="modal" | ||
| class="hidden fixed inset-0 z-[9999] flex items-center justify-center p-4" | ||
| aria-modal="true" | ||
| role="dialog" | ||
| > | ||
| <!-- Backdrop --> | ||
| <div class="absolute inset-0 bg-black/70"></div> | ||
|
|
||
| <!-- Modal panel --> | ||
| <div class="relative w-full max-w-2xl rounded-2xl bg-darkless p-6 shadow-2xl border border-red-500/30"> | ||
| <h2 class="text-xl font-semibold text-white"> | ||
| REQUIRED UPDATE (MANDATORY) | ||
| </h2> | ||
|
|
||
| <p class="mt-3 text-zinc-200"> | ||
| You must install or upgrade to the newest version of <strong>Godot Super WakaTime</strong>: | ||
| <br /> | ||
| <a class="underline break-all" href="https://github.com/BudzioT/Godot_Super-Wakatime" target="_blank" rel="noreferrer"> | ||
| https://github.com/BudzioT/Godot_Super-Wakatime | ||
| </a> | ||
| </p> | ||
|
|
||
| <p class="mt-3 text-zinc-200"> | ||
| <strong>Effective January 1, 2026</strong>, any Hackatime project that uses the old Godot time-tracking | ||
| plugin/extension will be <strong>immediately rejected</strong>. | ||
| </p> | ||
|
|
||
| <p class="mt-3 text-zinc-200"> | ||
| If you installed this plugin before, you still need to update. Do not assume you are on the newest version. | ||
| </p> | ||
|
|
||
| <div class="mt-6 flex flex-wrap items-center justify-end gap-3"> | ||
| <a | ||
| class="rounded-lg bg-white px-4 py-2 text-sm font-semibold text-black" | ||
| href="https://github.com/BudzioT/Godot_Super-Wakatime" | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| > | ||
| Get the newest version | ||
| </a> | ||
|
|
||
| <button | ||
| type="button" | ||
| class="rounded-lg bg-zinc-800 px-4 py-2 text-sm font-semibold text-white hover:bg-zinc-700" | ||
| data-action="sitewide-notice#dismissModal" | ||
| > | ||
| I understand | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The
sitewide_notice_controller.jshas an emptyconnect()method, which prevents the sitewide notice modal from ever being displayed, rendering the feature non-functional.Severity: HIGH | Confidence: High
🔍 Detailed Analysis
The
connect()method insitewide_notice_controller.jsis empty, with its logic commented out. As this method is the entry point for Stimulus controllers, the sitewide notice modal and banner will never be displayed to users. The controller is correctly attached to the DOM via the_sitewide_notice.html.erbpartial on every page, but because the initialization logic inconnect()is disabled, the modal remains hidden indefinitely. This renders the entire sitewide notice feature non-functional, despite all the supporting HTML and controller methods being in place.💡 Suggested Fix
To re-enable the sitewide notice feature, uncomment the logic within the
connect()method insitewide_notice_controller.js. This will allow the controller to check if the modal has been dismissed and callopenModal()on page load. If the intention was to permanently remove the feature, the controller and the corresponding partial rendering in the application layout should be removed entirely.🤖 Prompt for AI Agent
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID:
7509002