Skip to content

Conversation

@tomerqodo
Copy link

@tomerqodo tomerqodo commented Dec 4, 2025

User description

Benchmark PR plausible#5890

Type: Clean (correct implementation)

Original PR Title: Make various UI improvements
Original PR Description: ### Changes

  • Fix invite modal z-index issues (search, button and avatar on top of overlay)
  • Improve invite modal design
Before After
CleanShot 2025-11-17 at 09 54 27@2x Localhost · 2 38pm · 11-17
  • Add hover state to stats bars on dashboard
CleanShot.2025-11-15.at.10.55.13.mp4
  • Improve feature gate design
Before After
CleanShot 2025-11-17 at 09 38 34@2x CleanShot 2025-11-17 at 09 38 15@2x
  • Improve trial upgrade CTA design
Before After
CleanShot 2025-11-15 at 10 27 40@2x CleanShot 2025-11-15 at 10 27 01@2x

Tests

  • This PR does not require tests

Dark mode


PR Type

Enhancement


Description

  • Improve feature gate overlay design with better visual hierarchy

  • Add hover states to dashboard stats bars with smooth transitions

  • Redesign invitation modal with cleaner layout and improved typography

  • Enhance trial upgrade CTA with yellow button theme and better styling

  • Fix modal z-index issues to ensure proper overlay stacking


Diagram Walkthrough

flowchart LR
  A["Feature Gate Overlay"] -->|"Improved design"| B["Better visual hierarchy"]
  C["Dashboard Stats Bars"] -->|"Add hover states"| D["Smooth transitions"]
  E["Invitation Modal"] -->|"Redesign layout"| F["Cleaner typography"]
  G["Trial Upgrade CTA"] -->|"Yellow button theme"| H["Enhanced styling"]
  I["Modal Z-index"] -->|"Fix stacking"| J["Proper visibility"]
Loading

File Walkthrough

Relevant files
Enhancement
10 files
billing.ex
Redesign feature gate overlay and upgrade CTA messaging   
+27/-16 
generic.ex
Add yellow button theme and improve disabled state transitions
+4/-2     
sites.ex
Redesign invitation modal with improved layout and messaging
+21/-28 
bar.js
Add rounded corners and color transition to bar elements 
+1/-1     
conversions.js
Add hover state color change to conversion stats bars       
+1/-1     
props.js
Add hover state color change to properties stats bars       
+1/-1     
index.js
Add hover state color changes to page stats bars                 
+3/-3     
source-list.js
Add hover state color changes to source stats bars             
+3/-3     
list.tsx
Add row hover effects and improve bar color transitions   
+3/-3     
_header.html.heex
Redesign trial notification with yellow button theme         
+13/-6   
Bug fix
1 files
prima_modal.ex
Fix modal z-index and adjust positioning for better visibility
+4/-4     
Tests
5 files
billing_test.exs
Update tests to verify feature gate overlay text content 
+8/-12   
notice_test.exs
Update test assertions for revised upgrade messaging         
+1/-1     
funnel_settings_test.exs
Update test assertions for simplified upgrade text             
+2/-2     
props_settings_test.exs
Update test assertions for simplified upgrade text             
+4/-4     
team_setup_test.exs
Update tests to verify feature gate overlay text content 
+3/-4     

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No auditing: The new UI gating and upgrade CTA rendering adds/changes user-facing flows without any
visible logging of critical actions or decisions (e.g., access locked vs granted), which
may be acceptable for purely presentational changes but cannot be confirmed from the diff
alone.

Referred Code
  :if={@locked?}
  id="feature-gate-overlay"
  class="absolute backdrop-blur-[8px] bg-white/70 dark:bg-gray-800/50 inset-0 flex justify-center items-center"
>
  <div class="px-6 flex flex-col items-center gap-y-3">
    <div class="flex-shrink-0 bg-white dark:bg-gray-700 max-w-max rounded-md p-2 border border-gray-200 dark:border-gray-600 text-indigo-500">
      <Heroicons.lock_closed solid class="size-6 -mt-px pb-px" />
    </div>
    <div class="flex flex-col gap-y-1.5 items-center">
      <h3 class="font-medium text-gray-900 dark:text-gray-100">
        Upgrade to unlock
      </h3>
      <span
        id="lock-notice"
        class="max-w-sm sm:max-w-md mb-2 text-sm text-gray-600 dark:text-gray-100/60 leading-normal text-center"
      >
        To access this feature,
        <.upgrade_call_to_action current_role={@current_role} current_team={@current_team} />
      </span>
    </div>
  </div>

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing guards: UI behavior changes (hover states, bars) do not include explicit handling for potential
edge cases like undefined metrics or empty lists in the new/modified code paths, which may
be handled elsewhere but is not visible in this diff.

Referred Code
    </div>
  )
}

function renderRow(listItem: TListItem) {
  return (
    <div key={listItem.name} style={{ minHeight: ROW_HEIGHT }}>
      <div
        className="group flex w-full items-center hover:bg-gray-100/60 dark:hover:bg-gray-850 rounded-sm transition-colors duration-150"
        style={{ marginTop: ROW_GAP_HEIGHT }}
      >
        {renderBarFor(listItem)}
        {renderMetricValuesFor(listItem)}
      </div>
    </div>
  )
}

function renderBarFor(listItem: TListItem) {
  const lightBackground = color || 'bg-green-50 group-hover:bg-green-100'
  const metricToPlot = metrics.find((metric) => metric.meta.plot)?.key


 ... (clipped 11 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix incorrect conditional logic display

Refactor the conditional logic to correctly display the "Welcome aboard!"
message only for non-owner roles, fixing a bug for owners with plan issues.

lib/plausible_web/live/sites.ex [674-683]

 <p class="text-sm text-gray-600 dark:text-gray-400 text-pretty">
   You've been added as <b class="capitalize">{@invitation.invitation.role}</b>
   to the {@site.domain} analytics dashboard.
-  <%= if !(Map.get(@invitation, :exceeded_limits) || Map.get(@invitation, :no_plan)) &&
-            @invitation.invitation.role == :owner do %>
-    If you accept the ownership transfer, you will be responsible for billing going forward.
+  <%= if @invitation.invitation.role == :owner do %>
+    <%= if !(Map.get(@invitation, :exceeded_limits) || Map.get(@invitation, :no_plan)) do %>
+      If you accept the ownership transfer, you will be responsible for billing going forward.
+    <% end %>
   <% else %>
     Welcome aboard!
   <% end %>
 </p>
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a logic bug where an owner with an invalid plan would see a "Welcome aboard!" message instead of the billing warning, and the proposed fix correctly separates the logic for owners and non-owners.

Medium
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants