Skip to content

Conversation

@SailReal
Copy link
Member

@SailReal SailReal commented Jan 9, 2025

No description provided.

@SailReal SailReal requested a review from tobihagemann January 9, 2025 18:37
@coderabbitai

This comment was marked as off-topic.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
hugo.toml (1)

119-119: Consider removing Plausible analytics when adding Umami

The CSP header now allows both Plausible and Umami analytics to run simultaneously. This raises several concerns:

  1. Dual tracking may impact user privacy and GDPR compliance
  2. Running two analytics services affects page load performance
  3. Increased network requests and JavaScript execution overhead

Consider migrating completely from Plausible to Umami instead of running both services in parallel. If this is a transitional phase, please document the migration timeline.

- Content-Security-Policy = "default-src 'none'; script-src 'self' 'unsafe-eval' https://umami.skymatic.de/ https://plausible.skymatic.de/ https://community.cryptomator.org/ https://js.stripe.com/ https://*.paddle.com/ https://www.google.com/ https://www.gstatic.com/; style-src 'self' 'unsafe-inline' https://*.paddle.com/; img-src 'self' data: https://static.cryptomator.org/ https://*.paddle.com/ https://paddle.s3.amazonaws.com/; connect-src 'self' https://api.cryptomator.org/ https://store.cryptomator.org/ https://umami.skymatic.de/ https://plausible.skymatic.de/ http://localhost:8787/; font-src 'self'; media-src https://static.cryptomator.org/; frame-src https://community.cryptomator.org/ https://js.stripe.com/ https://*.paddle.com/ https://www.google.com/; base-uri 'self'; form-action 'self' https://www.paypal.com/ https://www.coinpayments.net/; frame-ancestors 'none'"
+ Content-Security-Policy = "default-src 'none'; script-src 'self' 'unsafe-eval' https://umami.skymatic.de/ https://community.cryptomator.org/ https://js.stripe.com/ https://*.paddle.com/ https://www.google.com/ https://www.gstatic.com/; style-src 'self' 'unsafe-inline' https://*.paddle.com/; img-src 'self' data: https://static.cryptomator.org/ https://*.paddle.com/ https://paddle.s3.amazonaws.com/; connect-src 'self' https://api.cryptomator.org/ https://store.cryptomator.org/ https://umami.skymatic.de/ http://localhost:8787/; font-src 'self'; media-src https://static.cryptomator.org/; frame-src https://community.cryptomator.org/ https://js.stripe.com/ https://*.paddle.com/ https://www.google.com/; base-uri 'self'; form-action 'self' https://www.paypal.com/ https://www.coinpayments.net/; frame-ancestors 'none'"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 392b557 and a9f7cdd.

📒 Files selected for processing (2)
  • hugo.toml (1 hunks)
  • layouts/_default/baseof.html (1 hunks)
🔇 Additional comments (1)
layouts/_default/baseof.html (1)

81-81: Verify Umami script features parity with Plausible

The Plausible script includes additional features in its URL:

  • file-downloads
  • hash
  • outbound-links
  • tagged-events

Please ensure that Umami provides equivalent tracking capabilities for:

  1. File download tracking
  2. URL hash tracking
  3. Outbound link tracking
  4. Custom event tracking

If any features are missing, document the gaps and their impact on analytics data collection.

Also applies to: 86-86

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
assets/js/base.js (1)

3-9: Optimize the outbound link tracking implementation

The current implementation has several areas for improvement:

  1. The jQuery selector runs on all anchor tags, which might impact performance
  2. No error handling for invalid href attributes
  3. The code runs immediately on script load

Consider this optimized implementation:

-$('a').each(function() {
-  const $a = $(this);
-  if ($a[0].host !== window.location.host && !$a.attr('data-umami-event')) {
-    $a.attr('data-umami-event', 'outbound-link-click');
-    $a.attr('data-umami-event-url', $a.attr('href'));
-  }
-});
+$(document).ready(function() {
+  // Only target anchors with href attributes that don't already have Umami tracking
+  $('a[href]:not([data-umami-event])').each(function() {
+    const $a = $(this);
+    try {
+      const href = $a.attr('href');
+      // Skip if href is not a valid URL
+      if (!href || href.startsWith('#')) return;
+      
+      const url = new URL(href, window.location.href);
+      if (url.host !== window.location.host) {
+        $a.attr('data-umami-event', 'outbound-link-click');
+        $a.attr('data-umami-event-url', href);
+      }
+    } catch (e) {
+      console.warn('Invalid URL in anchor:', e);
+    }
+  });
+});
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9f7cdd and 226a4fd.

📒 Files selected for processing (2)
  • assets/js/404.js (1 hunks)
  • assets/js/base.js (1 hunks)
🔇 Additional comments (3)
assets/js/404.js (2)

1-2: Verify the dual tracking strategy

The code now tracks 404 events in both Plausible and Umami. Please confirm if this dual tracking is intentional, as it might affect analytics data accuracy.


2-2: 🛠️ Refactor suggestion

Move Umami tracking inside DOMContentLoaded event listener

The Umami tracking call should be inside the DOMContentLoaded event listener to ensure the DOM is fully loaded before tracking.

-document.addEventListener('DOMContentLoaded', function () { plausible('404', { props: { path: document.location.pathname } }); });
-umami.track('404', { path: document.location.pathname });
+document.addEventListener('DOMContentLoaded', function () {
+  plausible('404', { props: { path: document.location.pathname } });
+  umami.track('404', { path: document.location.pathname });
+});

Likely invalid or redundant comment.

assets/js/base.js (1)

3-9: Verify Umami script availability

The code assumes Umami's script is loaded and ready to use. Consider adding a check for Umami's availability.

@tobihagemann tobihagemann force-pushed the feature/add-umami branch 2 times, most recently from d8a65f1 to e8cbb0c Compare February 11, 2025 09:34
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.

Actionable comments posted: 11

🧹 Nitpick comments (3)
content/downloads/android.de.html (1)

22-22: Consider removing legacy Plausible attributes.

The code contains both Plausible and Umami tracking attributes. Since you're migrating to Umami, the plausible-event-name attributes should be removed to avoid confusion and maintain clean code.

-      <a href="https://play.google.com/store/apps/details?id=org.cryptomator&hl=de" role="button" class="plausible-event-name=downloads-android-googleplay" data-umami-event="downloads-android-googleplay">
+      <a href="https://play.google.com/store/apps/details?id=org.cryptomator&hl=de" role="button" data-umami-event="downloads-android-googleplay">
-      <a href="/de/android/" role="button" class="btn btn-primary w-full md:w-48 plausible-event-name=downloads-android-apk" data-umami-event="downloads-android-apk">Zur APK-Version</a>
+      <a href="/de/android/" role="button" class="btn btn-primary w-full md:w-48" data-umami-event="downloads-android-apk">Zur APK-Version</a>

Also applies to: 34-34

layouts/partials/supporter-cert-influencer.html (1)

17-17: Consider using consistent event names across analytics platforms.

The Plausible event name is supporter-cert-influencer-form while the Umami event name is supporter-cert-influencer-submit. Using different names for the same event across analytics platforms could make tracking and analysis more difficult.

-      <button x-show="submitData.captcha" :disabled="feedbackData.inProgress || !acceptTerms" @click.prevent="new SupporterCertificate($refs.form, feedbackData, submitData).request()" type="submit" class="btn btn-primary w-full md:w-64 plausible-event-name=supporter-cert-influencer-form" data-umami-event="supporter-cert-influencer-submit" x-cloak>
+      <button x-show="submitData.captcha" :disabled="feedbackData.inProgress || !acceptTerms" @click.prevent="new SupporterCertificate($refs.form, feedbackData, submitData).request()" type="submit" class="btn btn-primary w-full md:w-64 plausible-event-name=supporter-cert-influencer-form" data-umami-event="supporter-cert-influencer-form" x-cloak>
content/downloads/linux.de.html (1)

30-33: Consider documenting the dual tracking strategy.

The code maintains both plausible-event-name and data-umami-event attributes. While this might be intentional during the transition from Plausible to Umami, it would be helpful to document this approach and plan for eventual cleanup.

Also applies to: 46-48

🛑 Comments failed to post (11)
layouts/partials/donate-paypal.html (1)

8-8: 🛠️ Refactor suggestion

Remove legacy Plausible tracking attribute.

The plausible-event-name class should be removed since we're migrating to Umami analytics.

-    <button type="submit" class="btn btn-primary w-full md:w-64 plausible-event-name=donate-paypal-link" data-umami-event="donate-paypal-link" :disabled="!acceptTerms"><i class="fas fa-external-link" aria-hidden="true"></i> {{ i18n "donate_paypal_calltoaction" }}</button>
+    <button type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-paypal-link" :disabled="!acceptTerms"><i class="fas fa-external-link" aria-hidden="true"></i> {{ i18n "donate_paypal_calltoaction" }}</button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    <button type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-paypal-link" :disabled="!acceptTerms"><i class="fas fa-external-link" aria-hidden="true"></i> {{ i18n "donate_paypal_calltoaction" }}</button>
content/downloads/ios.en.html (1)

22-22: 🛠️ Refactor suggestion

Remove legacy Plausible tracking attribute.

The plausible-event-name class should be removed since we're migrating to Umami analytics.

-      <a href="https://apps.apple.com/us/app/cryptomator/id1560822163" role="button" class="plausible-event-name=downloads-ios-appstore" data-umami-event="downloads-ios-appstore">
+      <a href="https://apps.apple.com/us/app/cryptomator/id1560822163" role="button" data-umami-event="downloads-ios-appstore">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      <a href="https://apps.apple.com/us/app/cryptomator/id1560822163" role="button" data-umami-event="downloads-ios-appstore">
content/downloads/ios.de.html (1)

22-22: 🛠️ Refactor suggestion

Remove legacy Plausible tracking and maintain consistency with English version.

  1. The plausible-event-name class should be removed since we're migrating to Umami analytics.
  2. The mb-4 class is present in the German version but not in the English version. Consider maintaining consistency between language versions.
-      <a href="https://apps.apple.com/de/app/cryptomator/id1560822163" role="button" class="mb-4 plausible-event-name=downloads-ios-appstore" data-umami-event="downloads-ios-appstore">
+      <a href="https://apps.apple.com/de/app/cryptomator/id1560822163" role="button" data-umami-event="downloads-ios-appstore">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      <a href="https://apps.apple.com/de/app/cryptomator/id1560822163" role="button" data-umami-event="downloads-ios-appstore">
layouts/partials/android-accrescent.html (1)

16-16: 🛠️ Refactor suggestion

Remove legacy Plausible tracking attribute.

The plausible-event-name class should be removed since we're migrating to Umami analytics.

-      <a href="https://accrescent.app/app/org.cryptomator" role="button" class="plausible-event-name=downloads-android-accrescent" data-umami-event="downloads-android-accrescent">
+      <a href="https://accrescent.app/app/org.cryptomator" role="button" data-umami-event="downloads-android-accrescent">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      <a href="https://accrescent.app/app/org.cryptomator" role="button" data-umami-event="downloads-android-accrescent">
layouts/partials/donate-crypto.html (1)

15-15: 🛠️ Refactor suggestion

Remove legacy Plausible tracking attribute.

The plausible-event-name class should be removed since we're migrating to Umami analytics.

-    <button type="submit" class="btn btn-primary w-full md:w-64 plausible-event-name=donate-crypto-link" data-umami-event="donate-crypto-link" :disabled="!acceptTerms"><i class="fas fa-external-link" aria-hidden="true"></i> {{ i18n "donate_crypto_calltoaction" }}</button>
+    <button type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-crypto-link" :disabled="!acceptTerms"><i class="fas fa-external-link" aria-hidden="true"></i> {{ i18n "donate_crypto_calltoaction" }}</button>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    <button type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-crypto-link" :disabled="!acceptTerms"><i class="fas fa-external-link" aria-hidden="true"></i> {{ i18n "donate_crypto_calltoaction" }}</button>
layouts/donate/list.html (1)

23-23: 🛠️ Refactor suggestion

Remove redundant Plausible tracking attributes.

The buttons have both Plausible and Umami tracking attributes, which could lead to duplicate tracking. Since we're transitioning to Umami, remove the Plausible attributes.

Apply this diff to remove the redundant Plausible tracking attributes:

-            <button @click.prevent="location.hash = '{{ .Params.TabHash }}'" :class="{'border-white': showTab != '{{ .Params.TabHash }}', 'border-primary': showTab == '{{ .Params.TabHash }}'}" class="flex items-center justify-between bg-white border-l-4 rounded-sm shadow-sm text-left p-4 w-full plausible-event-name=donate-{{ substr .Params.TabHash 1 }}-tab" data-umami-event="donate-{{ substr .Params.TabHash 1 }}-tab">
+            <button @click.prevent="location.hash = '{{ .Params.TabHash }}'" :class="{'border-white': showTab != '{{ .Params.TabHash }}', 'border-primary': showTab == '{{ .Params.TabHash }}'}" class="flex items-center justify-between bg-white border-l-4 rounded-sm shadow-sm text-left p-4 w-full" data-umami-event="donate-{{ substr .Params.TabHash 1 }}-tab">

-        <button :disabled="feedbackData.inProgress" @click.prevent="submitData.quantity > 0 && submitData.quantity < 5 ? lowQuantityModalIsOpen = true : hubManaged.request()" type="submit" class="btn btn-primary w-full md:w-64 plausible-event-name=donate-customer-portal-form" data-umami-event="donate-customer-portal-form">
+        <button :disabled="feedbackData.inProgress" @click.prevent="submitData.quantity > 0 && submitData.quantity < 5 ? lowQuantityModalIsOpen = true : hubManaged.request()" type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="donate-customer-portal-form">

Also applies to: 51-51

content/sponsors.en.html (1)

67-67: 🛠️ Refactor suggestion

Remove redundant Plausible tracking attribute.

The anchor tag has both Plausible and Umami tracking attributes, which could lead to duplicate tracking. Since we're transitioning to Umami, remove the Plausible attribute.

Apply this diff to remove the redundant Plausible tracking attribute:

-  <a href="https://github.com/sponsors/cryptomator" target="_blank" rel="noopener" role="button" class="btn btn-primary px-8 plausible-event-name=sponsors-github-cta" data-umami-event="sponsors-github-cta">
+  <a href="https://github.com/sponsors/cryptomator" target="_blank" rel="noopener" role="button" class="btn btn-primary px-8" data-umami-event="sponsors-github-cta">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  <a href="https://github.com/sponsors/cryptomator" target="_blank" rel="noopener" role="button" class="btn btn-primary px-8" data-umami-event="sponsors-github-cta">
layouts/hub-managed/single.html (1)

90-90: 🛠️ Refactor suggestion

Remove redundant Plausible tracking attributes.

The button and anchor tag have both Plausible and Umami tracking attributes, which could lead to duplicate tracking. Since we're transitioning to Umami, remove the Plausible attributes.

Apply this diff to remove the redundant Plausible tracking attributes:

-        <button x-show="submitData.captcha" :disabled="feedbackData.inProgress || !acceptTerms" @click.prevent="submitData.quantity > 0 && submitData.quantity < 5 ? lowQuantityModalIsOpen = true : hubManaged.request()" type="submit" class="btn btn-primary w-full md:w-64 plausible-event-name=hub-managed-form" data-umami-event="hub-managed-form" x-cloak>
+        <button x-show="submitData.captcha" :disabled="feedbackData.inProgress || !acceptTerms" @click.prevent="submitData.quantity > 0 && submitData.quantity < 5 ? lowQuantityModalIsOpen = true : hubManaged.request()" type="submit" class="btn btn-primary w-full md:w-64" data-umami-event="hub-managed-form" x-cloak>

-    <a href="mailto:[email protected]" role="button" class="btn btn-outline-primary px-8 plausible-event-name=hub-managed-contact-us" data-umami-event="hub-managed-contact-us">
+    <a href="mailto:[email protected]" role="button" class="btn btn-outline-primary px-8" data-umami-event="hub-managed-contact-us">

Also applies to: 103-103

layouts/supporter-cert/list.html (1)

23-23: 🛠️ Refactor suggestion

Remove redundant Plausible tracking attributes.

The buttons have both Plausible and Umami tracking attributes, which could lead to duplicate tracking. Since we're transitioning to Umami, remove the Plausible attributes.

Apply this diff to remove the redundant Plausible tracking attributes:

-            <button @click.prevent="location.hash = '{{ .Params.TabHash }}'" :class="{'border-white': showTab != '{{ .Params.TabHash }}', 'border-primary': showTab == '{{ .Params.TabHash }}'}" class="flex items-center justify-between bg-white border-l-4 rounded-sm shadow-sm text-left p-4 w-full plausible-event-name=supporter-cert-{{ substr .Params.TabHash 1 }}-tab" data-umami-event="supporter-cert-{{ substr .Params.TabHash 1 }}-tab">
+            <button @click.prevent="location.hash = '{{ .Params.TabHash }}'" :class="{'border-white': showTab != '{{ .Params.TabHash }}', 'border-primary': showTab == '{{ .Params.TabHash }}'}" class="flex items-center justify-between bg-white border-l-4 rounded-sm shadow-sm text-left p-4 w-full" data-umami-event="supporter-cert-{{ substr .Params.TabHash 1 }}-tab">

-        <button :disabled="data.inProgress" @click.prevent="new KeyRecovery($refs.form, data).getUserHistory()" type="submit" class="shrink-0 flex items-center gap-1 btn btn-primary rounded-l-none px-4 plausible-event-name=supporter-cert-recovery-form" data-umami-event="supporter-cert-recovery-form">
+        <button :disabled="data.inProgress" @click.prevent="new KeyRecovery($refs.form, data).getUserHistory()" type="submit" class="shrink-0 flex items-center gap-1 btn btn-primary rounded-l-none px-4" data-umami-event="supporter-cert-recovery-form">

Also applies to: 54-54

layouts/hub/single.html (1)

17-17: 🛠️ Refactor suggestion

Remove redundant Plausible tracking attributes.

Multiple elements have both Plausible and Umami tracking attributes, which could lead to duplicate tracking. Since we're transitioning to Umami, remove the Plausible attributes.

Apply this diff to remove the redundant Plausible tracking attributes:

-            <a href="{{ .Site.LanguagePrefix }}/hub/managed/" role="button" class="btn btn-primary text-lg px-8 plausible-event-name=hub-header-managed-cta" data-umami-event="hub-header-managed-cta">{{ i18n "hub_header_managed_cta" . }}</a>
+            <a href="{{ .Site.LanguagePrefix }}/hub/managed/" role="button" class="btn btn-primary text-lg px-8" data-umami-event="hub-header-managed-cta">{{ i18n "hub_header_managed_cta" . }}</a>

-            <a href="{{ .Site.LanguagePrefix }}/hub/self-hosted/" role="button" class="btn btn-primary text-lg px-8 plausible-event-name=hub-header-self-hosted-cta" data-umami-event="hub-header-self-hosted-cta">{{ i18n "hub_header_self_hosted_cta" . }}</a>
+            <a href="{{ .Site.LanguagePrefix }}/hub/self-hosted/" role="button" class="btn btn-primary text-lg px-8" data-umami-event="hub-header-self-hosted-cta">{{ i18n "hub_header_self_hosted_cta" . }}</a>

-      <a href="{{ .Site.LanguagePrefix }}/pricing/#for-teams" role="button" class="btn btn-primary px-8 plausible-event-name=hub-pricing" data-umami-event="hub-pricing">
+      <a href="{{ .Site.LanguagePrefix }}/pricing/#for-teams" role="button" class="btn btn-primary px-8" data-umami-event="hub-pricing">

-    <a href="mailto:[email protected]" role="button" class="btn btn-outline-primary px-8 plausible-event-name=hub-contact-us" data-umami-event="hub-contact-us">
+    <a href="mailto:[email protected]" role="button" class="btn btn-outline-primary px-8" data-umami-event="hub-contact-us">

-        <button :disabled="data.inProgress || !data.acceptTerms" @click.prevent="new Newsletter($refs.form, data).subscribe()" type="submit" class="shrink-0 flex items-center gap-1 btn btn-primary rounded-l-none px-4 plausible-event-name=hub-newsletter-form" data-umami-event="hub-newsletter-form">
+        <button :disabled="data.inProgress || !data.acceptTerms" @click.prevent="new Newsletter($refs.form, data).subscribe()" type="submit" class="shrink-0 flex items-center gap-1 btn btn-primary rounded-l-none px-4" data-umami-event="hub-newsletter-form">

Also applies to: 21-21, 75-75, 105-105, 117-117

layouts/partials/nav.html (1)

134-135: ⚠️ Potential issue

Fix the template syntax error.

There is an unexpected {{end}} tag at line 135 that is causing the pipeline to fail.

Apply this diff to fix the template syntax:

-    {{- end }}
-    {{- end }}
+    {{- end }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    {{- end }}
🧰 Tools
🪛 GitHub Actions: GitHub Pages

[error] 135-135: Unexpected {{end}} in template. This indicates a potential mismatch in template tags.

@tobihagemann tobihagemann merged commit d29ee69 into develop Feb 11, 2025
4 checks passed
@tobihagemann tobihagemann deleted the feature/add-umami branch February 11, 2025 09:58
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