Skip to content
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

enhance(apps/docs): add about page #4376

Merged
merged 16 commits into from
Jan 2, 2025
Merged

enhance(apps/docs): add about page #4376

merged 16 commits into from
Jan 2, 2025

Conversation

Queentaker
Copy link
Contributor

@Queentaker Queentaker commented Nov 14, 2024

  • routing needs to be added
  • and added iframe to development

Summary by CodeRabbit

  • New Features
    • Added an "About" page for KlickerUZH, featuring:
      • Introduction to the platform
      • Embedded video
      • Team members section
      • Project sponsors showcase
  • Documentation
    • Updated site navigation to include new "About" page
  • Style
    • Implemented responsive design using Tailwind CSS

Copy link

aviator-app bot commented Nov 14, 2024

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.


See the real-time status of this PR on the Aviator webapp.
Use the Aviator Chrome Extension to see the status of your PR within GitHub.

Copy link

coderabbitai bot commented Nov 14, 2024

📝 Walkthrough

Walkthrough

This pull request introduces updates to the documentation website for KlickerUZH, focusing on adding an "About" page and refining the development page. The changes include creating a new About component in about.tsx that provides detailed information about the platform, its team, and sponsors. The documentation configuration is updated to include a new navigation item linking to the About page. Additionally, a minor enhancement is made to the development page by using a design system component for the page heading.

Changes

File Change Summary
apps/docs/src/pages/about.tsx Added new React component About with:
- Introduction to KlickerUZH platform
- Embedded video
- Team members list with dynamic rendering
- Project sponsors section
apps/docs/src/pages/development.tsx Replaced div title with H1 component from design system
apps/docs/docusaurus.config.ts Added new "About" navigation item linking to /about path

Possibly related PRs


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@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: 6

🧹 Outside diff range and nitpick comments (5)
apps/docs/src/pages/development.tsx (1)

148-150: Enhance video section description

The current description "Video Explanation" is too generic. Consider adding more context about the video's content.

Suggested improvement:

 <div className="m-0 flex flex-col items-center pb-0">
-  <p className="mb-2"> Video Explanation</p>
+  <p className="mb-2">Overview of KlickerUZH Development Process and Goals</p>
 </div>
apps/docs/src/pages/about.tsx (4)

13-29: Consider externalizing metrics data.

The metrics are currently hardcoded within the component. Consider moving this data to a separate configuration file for easier maintenance and updates.

+ // metrics.config.ts
+ export const kpiMetrics = [
+   {
+     value: '1894',
+     label: 'Lecturers',
+     description: 'use KlickerUZH',
+   },
+   // ... other metrics
+ ]

60-91: Consider extracting feature list into a reusable component.

The feature lists for both Standard and Catalyst tiers share similar structure. Consider extracting this into a reusable component to reduce code duplication.

interface Feature {
  text: string;
  icon?: IconDefinition;
}

interface FeatureListProps {
  features: Feature[];
}

const FeatureList: React.FC<FeatureListProps> = ({ features }) => (
  <ul className="mb-2 mt-8 space-y-2 pl-0">
    {features.map((feature, index) => (
      <li key={index} className="flex gap-x-3">
        <div>
          <FontAwesomeIcon icon={feature.icon ?? faCheck} />
        </div>
        <div>{feature.text}</div>
      </li>
    ))}
  </ul>
);

Also applies to: 98-139


158-158: Remove empty flex containers.

There are empty flex containers that serve no purpose and should be removed.

  <h1 className="mt-2 flex w-max flex-row gap-4 md:text-5xl">
    <div>Purpose of KlickerUZH</div>
-   <div className="flex justify-center"></div>
  </h1>

  <h1 className="mt-2 flex w-max flex-row gap-4 md:text-5xl">
    <div>Team</div>
-   <div className="flex justify-center"></div>
  </h1>

Also applies to: 174-174


203-209: DRY: Extract form link to a constant.

The form link is duplicated from the Models component. Consider extracting it to a constant to maintain DRY principles.

+ const INQUIRY_FORM_URL = 'https://forms.office.com/e/4AsWW1uck2';

// Then use INQUIRY_FORM_URL in both locations
<a 
  href={INQUIRY_FORM_URL}
  target="_blank"
  rel="noopener noreferrer"
>
  form
</a>
🛑 Comments failed to post (6)
apps/docs/src/pages/development.tsx (2)

140-147: 🛠️ Refactor suggestion

Enhance iframe security and accessibility

The iframe implementation needs improvements in several areas:

  1. Add sandbox attribute to restrict iframe capabilities
  2. Add loading="lazy" for performance
  3. Use a more descriptive title
  4. Consider error handling for video loading failures

Apply these improvements:

 <iframe
   src="https://api.cast.switch.ch/p/106/embedPlaykitJs/uiconf_id/23449004/partner_id/106?iframeembed=true&playerId=kaltura_player&entry_id=0_ol91rao1"
   width="100%"
   height="360"
   allowFullScreen
   allow="fullscreen"
-  title="Video Player"
+  title="KlickerUZH Development Overview Video"
+  loading="lazy"
+  sandbox="allow-scripts allow-same-origin allow-popups"
+  onError={(e) => console.error('Failed to load video player:', e)}
 />
📝 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.

            <iframe
              src="https://api.cast.switch.ch/p/106/embedPlaykitJs/uiconf_id/23449004/partner_id/106?iframeembed=true&playerId=kaltura_player&entry_id=0_ol91rao1"
              width="100%"
              height="360"
              allowFullScreen
              allow="fullscreen"
              title="KlickerUZH Development Overview Video"
              loading="lazy"
              sandbox="allow-scripts allow-same-origin allow-popups"
              onError={(e) => console.error('Failed to load video player:', e)}
            />

138-152: 💡 Codebase verification

Video configuration should be centralized

Based on the search results, there are multiple video embeds across the documentation and frontend with hardcoded configuration:

  • Different uiConfId values are used (23448425 and 23449004)
  • Multiple video URLs follow two different patterns:
    • api.cast.switch.ch/p/106/embedPlaykitJs/... (in development.tsx)
    • uzh.mediaspace.cast.switch.ch/embed/secure/iframe/... (in other files)
  • Configuration IDs like partner_id and entry_id are hardcoded in each file

Recommendation:

  • Create a centralized configuration for the video player settings
  • Consider environment-specific configurations for different environments (development, production)
  • Standardize the video URL pattern across all embeds
🔗 Analysis chain

Consider environment configuration for video source

The video URL contains specific IDs (partner_id, entry_id, etc.) that might need to be configurable based on the environment.

Let's check if these IDs are used elsewhere or if they should be moved to configuration:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for cast.switch.ch URLs or similar video player configurations
rg -l "cast.switch.ch|entry_id|partner_id" --type=typescript --type=javascript

Length of output: 115


Script:

#!/bin/bash
# Search for cast.switch.ch URLs or similar video player configurations with proper file extensions
rg -l "cast\.switch\.ch|entry_id|partner_id" --type=ts --type=tsx --type=js --type=jsx

Length of output: 116


Script:

#!/bin/bash
# Search for cast.switch.ch URLs or similar video player configurations in all files
rg "cast\.switch\.ch|entry_id|partner_id" -l

Length of output: 602


Script:

#!/bin/bash
# Let's check the actual content to see how these IDs are used in other files
rg "cast\.switch\.ch|entry_id|partner_id" -A 2 -B 2

Length of output: 7960


Script:

#!/bin/bash
# Let's check if there are any environment variables or configuration files that might be related to these video settings
rg -l "CAST_SWITCH|VIDEO|MEDIA|KALTURA" --type=env --type=json --type=yaml

Length of output: 104

apps/docs/src/pages/about.tsx (4)

171-171: 🛠️ Refactor suggestion

Use proper JSX syntax for component rendering.

Components are being invoked directly as functions instead of using JSX syntax.

- {Purpose}
+ <Purpose />

- {Models()}
+ <Models />

- {KPIs()}
+ <KPIs />

Also applies to: 184-184, 211-211


143-145: ⚠️ Potential issue

Add security attributes to external link.

External links should include proper security attributes to prevent potential security vulnerabilities.

- <a href="https://forms.office.com/e/4AsWW1uck2" target="_blank">
+ <a 
+   href="https://forms.office.com/e/4AsWW1uck2" 
+   target="_blank" 
+   rel="noopener noreferrer"
+ >
📝 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://forms.office.com/e/4AsWW1uck2" 
              target="_blank" 
              rel="noopener noreferrer"
            >
              form
            </a>

37-46: 🛠️ Refactor suggestion

Enhance accessibility with ARIA labels.

The metrics cards lack proper accessibility attributes. Consider adding appropriate ARIA labels and roles.

 <div
   key={index}
   className="bg-uzh-blue-80 flex flex-col items-center justify-center rounded-lg p-6 text-center text-white"
+  role="region"
+  aria-label={`${metric.label} metric`}
 >
📝 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.

        {metrics.map((metric, index) => (
          <div
            key={index}
            className="bg-uzh-blue-80 flex flex-col items-center justify-center rounded-lg p-6 text-center text-white"
            role="region"
            aria-label={`${metric.label} metric`}
          >
            <div className="text-3xl font-bold">{metric.value}</div>
            <div className="mt-2 text-xl font-semibold">{metric.label}</div>
            <div className="mt-1 text-sm">{metric.description}</div>
          </div>
        ))}

154-164: ⚠️ Potential issue

Refactor Purpose component and replace placeholder text.

The Purpose component has two issues:

  1. It's incorrectly defined as a constant with JSX fragment instead of a proper function component
  2. Contains Lorem ipsum placeholder text that needs to be replaced with actual content
- const Purpose = (
-   <>
+ const Purpose: React.FC = () => (
+   <div className="purpose-section">
      <h1 className="mt-2 flex w-max flex-row gap-4 md:text-5xl">
        <div>Purpose of KlickerUZH</div>
        <div className="flex justify-center"></div>
      </h1>
      <p className="text-center">
-       Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do
+       {/* TODO: Add actual purpose description */}
      </p>
-   </>
+   </div>
  )
📝 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.

const Purpose: React.FC = () => (
  <div className="purpose-section">
    <h1 className="mt-2 flex w-max flex-row gap-4 md:text-5xl">
      <div>Purpose of KlickerUZH</div>
      <div className="flex justify-center"></div>
    </h1>
    <p className="text-center">
      {/* TODO: Add actual purpose description */}
    </p>
  </div>
)

Copy link

cypress bot commented Nov 14, 2024

klicker-uzh    Run #3903

Run Properties:  status check passed Passed #3903  •  git commit be944f573a ℹ️: Merge 3c35c3496503b930bc5713b017af1ae08fc693e2 into 2212bd25fb6433a83a6f29203475...
Project klicker-uzh
Branch Review docs_about_page
Run status status check passed Passed #3903
Run duration 13m 43s
Commit git commit be944f573a ℹ️: Merge 3c35c3496503b930bc5713b017af1ae08fc693e2 into 2212bd25fb6433a83a6f29203475...
Committer Chris
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 172
View all changes introduced in this branch ↗︎

Copy link
Member

@rschlaefli rschlaefli left a comment

Choose a reason for hiding this comment

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

Have not looked at the contents yet -> please remove the .idea folder and the func-migration folders, as they should not be committed or have been removed

@rschlaefli rschlaefli changed the title Docs about page enhance(apps/docs): add about page Nov 18, 2024
Copy link

@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: 1

🛑 Comments failed to post (1)
apps/docs/docusaurus.config.ts (1)

158-162: 💡 Codebase verification

The about page exists but is incorrectly named as Catalyst

The file at apps/docs/src/pages/about.tsx exists but is incorrectly named as Catalyst component. This creates confusion as there's another Catalyst component in catalyst.tsx. The about page should be renamed to About to match its route and avoid naming conflicts.

  • apps/docs/src/pages/about.tsx: Rename the component from Catalyst to About
  • Consider moving the models and pricing information to catalyst.tsx as it seems more relevant there
🔗 Analysis chain

Verify the about page implementation.

Please ensure that the corresponding about page component exists at apps/docs/src/pages/about.tsx as mentioned in the AI summary.


The about page exists but there might be a naming conflict with the Catalyst component

The verification reveals that about.tsx exists at the correct location, but there are two components named Catalyst - one in about.tsx and another in catalyst.tsx. Let's verify if this is intentional by checking their implementations.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of the about page component and its imports

# Check if the about page component exists
fd "about.tsx" "apps/docs/src/pages"

# Check for Catalyst component references
rg -A 5 "Catalyst" "apps/docs/src/pages"

Length of output: 1969


Script:

#!/bin/bash
# Check the full content of both files to understand their relationship
cat apps/docs/src/pages/about.tsx
cat apps/docs/src/pages/catalyst.tsx

Length of output: 15553

who else to add, ensuring image scales as well.
Copy link

@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: 6

🧹 Outside diff range and nitpick comments (1)
apps/docs/src/pages/about.tsx (1)

67-167: Improve Models component structure and accessibility

The Models component could benefit from better semantic structure and accessibility.

  1. Add proper semantic structure:
- <div className="space-y-4 rounded-lg bg-slate-100 p-6 sm:rounded-xl sm:p-8">
+ <section aria-labelledby="model-standard" className="space-y-4 rounded-lg bg-slate-100 p-6 sm:rounded-xl sm:p-8">
+   <h2 id="model-standard" className="text-4xl font-semibold tracking-tight md:text-5xl">
    Standard
+   </h2>
  1. Consider extracting feature lists to a configuration file for better maintainability.
🛑 Comments failed to post (6)
apps/docs/src/pages/about.tsx (6)

27-65: 🛠️ Refactor suggestion

Add TypeScript interfaces and improve accessibility

The KPIs component needs type safety and better accessibility support.

  1. Add TypeScript interface:
interface Metric {
  value: string
  label: string
  description: string
}
  1. Add aria labels and roles:
  <div
    key={index}
+   role="article"
+   aria-label={`${metric.label}: ${metric.value} ${metric.description}`}
    className="bg-uzh-blue-80 flex flex-col items-center justify-center rounded-lg p-6 text-center text-white"
  >
  1. Consider moving metrics data to a configuration file:
// config/metrics.ts
export const metrics: Metric[] = [
  {
    value: '1894',
    label: 'Lecturers',
    description: 'use KlickerUZH',
  },
  // ...
]

242-262: 🛠️ Refactor suggestion

Improve About component composition and error handling

The main About component needs improvements in component composition and error handling.

  1. Fix Purpose component usage:
- {Purpose}
+ <Purpose />
  1. Add error boundary:
import { ErrorBoundary } from '@/components/ErrorBoundary'

function About() {
  return (
    <Layout>
      <ErrorBoundary>
        <div className="flex max-w-7xl flex-col items-center text-center md:mx-auto lg:px-8">
          {/* ... */}
        </div>
      </ErrorBoundary>
    </Layout>
  )
}
  1. Consider adding loading states for dynamic content

158-160: ⚠️ Potential issue

Enhance external link security and accessibility

The external form link needs security and accessibility improvements.

  <a 
    href="https://forms.office.com/e/4AsWW1uck2" 
    target="_blank"
+   rel="noopener noreferrer"
+   aria-label="Access request form (opens in new tab)"
  >
    form
  </a>
📝 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://forms.office.com/e/4AsWW1uck2" 
              target="_blank"
              rel="noopener noreferrer"
              aria-label="Access request form (opens in new tab)"
            >
              form
            </a>

11-25: 🛠️ Refactor suggestion

Consider externalizing team member data

The hardcoded team member data should be moved to a separate configuration file for better maintainability. Additionally, the image URLs point directly to production, which might not be ideal for development/staging environments.

Consider:

  1. Moving the data to a separate config file (e.g., config/team.ts)
  2. Using environment variables for the base URL of images
  3. Adding proper alt text descriptions for accessibility
- const people = [
-   {
-     name: 'Roland Schläfli',
-     imageUrl: 'https://www.df.uzh.ch/contacts/df/admin/teaching-center/rschl%C3%A4fli/photo/Schl%C3%A4fli-Roland.jpg.jpg',
-     githubUrl: 'https://github.com/rschaefli',
-   },
-   // ...
- ]
+ import { team } from '@/config/team'

Committable suggestion skipped: line range outside the PR's diff.


169-178: ⚠️ Potential issue

Fix Purpose component implementation and add real content

The Purpose component has several issues:

  1. Incorrect implementation as a JSX fragment instead of a proper function component
  2. Contains Lorem ipsum placeholder text
  3. Missing semantic structure
- const Purpose = (
-  <>
-    <h1 className="mt-2 flex w-max flex-row gap-4 md:text-5xl">
-      <div> Purpose of KlickerUZH</div>
-    </h1>
-    <p className="text-left">
-      Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed do
-    </p>
-  </>
- )
+ function Purpose() {
+   return (
+     <section aria-labelledby="purpose-title">
+       <h1 id="purpose-title" className="mt-2 flex w-max flex-row gap-4 md:text-5xl">
+         Purpose of KlickerUZH
+       </h1>
+       <p className="text-left">
+         {/* TODO: Add actual content describing the purpose of KlickerUZH */}
+       </p>
+     </section>
+   )
+ }

Committable suggestion skipped: line range outside the PR's diff.


180-240: 🛠️ Refactor suggestion

Optimize images and enhance accessibility

The Team component needs improvements in image handling and accessibility.

  1. Use Next.js Image component for optimization:
- <img
-   alt=""
-   src={person.imageUrl}
-   className="h-full w-full object-cover"
- />
+ <Image
+   alt={`${person.name}'s profile picture`}
+   src={person.imageUrl}
+   width={64}
+   height={64}
+   className="h-full w-full object-cover"
+ />
  1. Add proper aria labels for GitHub links:
  <a
    href={person.githubUrl}
    target="_blank"
    rel="noopener noreferrer"
+   aria-label={`${person.name}'s GitHub profile (opens in new tab)`}
    className="text-gray-500 hover:text-gray-700"
  >
📝 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.

function Team() {
  return (
    <div className="bg-white py-24 sm:py-32">
      <div className="mx-auto grid max-w-7xl gap-20 px-6 lg:px-8 xl:grid-cols-3">
        <div className="max-w-xl">
          <h2 className="text-pretty text-3xl font-semibold tracking-tight text-gray-900 sm:text-4xl">
            Team
          </h2>
          <p className="mt-6 text-lg/8 text-gray-600">
            We're a dynamic group of individuals who are passionate about what
            we do and dedicated to delivering the best results for our clients.
          </p>
        </div>
        <ul
          role="list"
          className="grid list-none gap-x-8 gap-y-12 sm:grid-cols-2 sm:gap-y-16 xl:col-span-2"
        >
          {people.map((person) => (
            <li key={person.name}>
              <div className="flex items-center gap-x-6">
                <div className="h-16 w-16 flex-shrink-0 overflow-hidden rounded-full">
                  <Image
                    alt={`${person.name}'s profile picture`}
                    src={person.imageUrl}
                    width={64}
                    height={64}
                    className="h-full w-full object-cover"
                  />
                </div>
                <div>
                  <h3 className="text-base/7 font-semibold tracking-tight text-gray-900">
                    {person.name}
                  </h3>
                  <div className="flex items-center gap-x-2">
                    <a
                      href={person.githubUrl}
                      target="_blank"
                      rel="noopener noreferrer"
                      aria-label={`${person.name}'s GitHub profile (opens in new tab)`}
                      className="text-gray-500 hover:text-gray-700"
                    >
                      <svg
                        className="h-5 w-5"
                        fill="currentColor"
                        viewBox="0 0 24 24"
                        aria-hidden="true"
                      >
                        <path
                          fillRule="evenodd"
                          d="M12 2C6.477 2 2 6.484 2 12.017c0 4.425 2.865 8.18 6.839 9.504.5.092.682-.217.682-.483 0-.237-.008-.868-.013-1.703-2.782.605-3.369-1.343-3.369-1.343-.454-1.158-1.11-1.466-1.11-1.466-.908-.62.069-.608.069-.608 1.003.07 1.531 1.032 1.531 1.032.892 1.53 2.341 1.088 2.91.832.092-.647.35-1.088.636-1.338-2.22-.253-4.555-1.113-4.555-4.951 0-1.093.39-1.988 1.029-2.688-.103-.253-.446-1.272.098-2.65 0 0 .84-.27 2.75 1.026A9.564 9.564 0 0112 6.844c.85.004 1.705.115 2.504.337 1.909-1.296 2.747-1.027 2.747-1.027.546 1.379.202 2.398.1 2.651.64.7 1.028 1.595 1.028 2.688 0 3.848-2.339 4.695-4.566 4.943.359.309.678.92.678 1.855 0 1.338-.012 2.419-.012 2.747 0 .268.18.58.688.482A10.019 10.019 0 0022 12.017C22 6.484 17.522 2 12 2z"
                          clipRule="evenodd"
                        />
                      </svg>
                    </a>
                  </div>
                </div>
              </div>
            </li>
          ))}
        </ul>
      </div>
    </div>
  )
}

Copy link

@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: 4

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 308da86 and 498e171.

📒 Files selected for processing (2)
  • apps/docs/src/pages/about.tsx (1 hunks)
  • apps/docs/src/pages/development.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/docs/src/pages/development.tsx
🔇 Additional comments (2)
apps/docs/src/pages/about.tsx (2)

1-2: LGTM! Clean imports following project conventions.

The imports are well-organized and properly utilize the project's design system components.


148-148: LGTM! Clean export statement.

The export follows React conventions.

Copy link

@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

♻️ Duplicate comments (2)
apps/docs/src/pages/about.tsx (2)

4-4: 🛠️ Refactor suggestion

Enhance type safety with a comprehensive interface.

The current type definition is minimal and doesn't capture all the necessary information about team members.

Consider implementing a more robust interface:

interface TeamMember {
  name: string;
  role: string;
  imageUrl?: string;
  contactUrl?: string;
  isAlumnus: boolean;
  department?: string;
}

154-157: 🛠️ Refactor suggestion

Add alt text to sponsor logos.

The sponsor logos are missing alt text, which is important for accessibility.

 <div className="mb-4 flex h-12 flex-row items-start">
-  <img src="/img/logo_swissuniversities.png" className="mr-8 h-full" />
-  <img src="/img/logo_uzh.jpeg" className="h-full" />
+  <img 
+    src="/img/logo_swissuniversities.png" 
+    alt="swissuniversities logo" 
+    className="mr-8 h-full" 
+  />
+  <img 
+    src="/img/logo_uzh.jpeg" 
+    alt="University of Zurich logo" 
+    className="h-full" 
+  />
 </div>
🧹 Nitpick comments (1)
apps/docs/src/pages/about.tsx (1)

48-90: Standardize data structure for team members without profile images.

Several team members are missing profile images and contact information. Consider:

  1. Adding profile images for all current team members
  2. Using a consistent format for alumni entries
  3. Including contact information where appropriate
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4e6d480 and 3c35c34.

⛔ Files ignored due to path filters (1)
  • apps/docs/static/img_v3/anonymousUser.svg is excluded by !**/*.svg
📒 Files selected for processing (1)
  • apps/docs/src/pages/about.tsx (1 hunks)
🔇 Additional comments (2)
apps/docs/src/pages/about.tsx (2)

1-2: LGTM! Appropriate imports are used.

The imports from theme and design system components are correctly utilized.


118-124: Enhance video iframe accessibility and performance.

The video iframe could benefit from additional attributes for better accessibility and performance.

 <iframe
   src="https://api.cast.switch.ch/p/106/embedPlaykitJs/uiconf_id/23449004/partner_id/106?iframeembed=true&playerId=kaltura_player&entry_id=0_ol91rao1"
   className="aspect-video w-full max-w-3xl border-2 border-solid border-black"
   allowFullScreen
   allow="fullscreen"
   title="Video Player"
+  loading="lazy"
+  aria-label="Introduction to KlickerUZH"
 />

Comment on lines +105 to +114
<span
onClick={() =>
window.open('https://github.com/uzh-bf/klicker-uzh', '_blank')
}
className="cursor-pointer text-blue-800 hover:underline"
>
entirely open-source
</span>
, allowing for further extensibility and collaboration.
</p>
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Replace onClick handler with proper link component.

Using raw onClick handlers for navigation is not recommended. Consider using a proper link component for better accessibility and SEO.

-<span
-  onClick={() =>
-    window.open('https://github.com/uzh-bf/klicker-uzh', '_blank')
-  }
-  className="cursor-pointer text-blue-800 hover:underline"
->
-  entirely open-source
-</span>
+<a
+  href="https://github.com/uzh-bf/klicker-uzh"
+  target="_blank"
+  rel="noopener noreferrer"
+  className="text-blue-800 hover:underline"
+>
+  entirely open-source
+</a>
📝 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.

Suggested change
<span
onClick={() =>
window.open('https://github.com/uzh-bf/klicker-uzh', '_blank')
}
className="cursor-pointer text-blue-800 hover:underline"
>
entirely open-source
</span>
, allowing for further extensibility and collaboration.
</p>
<a
href="https://github.com/uzh-bf/klicker-uzh"
target="_blank"
rel="noopener noreferrer"
className="text-blue-800 hover:underline"
>
entirely open-source
</a>
, allowing for further extensibility and collaboration.
</p>

Copy link

sonarqubecloud bot commented Jan 2, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
26.3% Duplication on New Code (required ≤ 3%)
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@sjschlapbach sjschlapbach merged commit 62d556a into v3 Jan 2, 2025
13 of 14 checks passed
@sjschlapbach sjschlapbach deleted the docs_about_page branch January 2, 2025 23:27
Copy link

cypress bot commented Jan 2, 2025

klicker-uzh    Run #3904

Run Properties:  status check passed Passed #3904  •  git commit 62d556a047: enhance(apps/docs): add about page (#4376)
Project klicker-uzh
Branch Review v3
Run status status check passed Passed #3904
Run duration 13m 28s
Commit git commit 62d556a047: enhance(apps/docs): add about page (#4376)
Committer Chris
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 172
View all changes introduced in this branch ↗︎

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

Successfully merging this pull request may close these issues.

3 participants