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

fix(theme): use ApiAnchor directly to render Api organism #103

Merged
merged 1 commit into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/theme/organisms/api/Api.vue
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
<script setup lang="ts">

import {computed, ref} from "vue";
import ApiList from "../../molecules/api-list/ApiList.vue";
import LightBanner from "../../molecules/banner/LightBanner.vue";
import {Search} from "lucide-vue-next";
import ButtonBoxes from "../../molecules/button-boxes/ButtonBoxes.vue";
import type {ApiResponse, ApiSymbol} from "../../composables/api/interfaces/Api";
import ApiAnchor from "../../molecules/api-anchor/ApiAnchor.vue";

interface Props extends ApiResponse {}
interface Props extends ApiResponse {
}

const {symbolTypes, modules: initialModules} = withDefaults(defineProps<Props>(), {});

const q = ref("");
const category = ref("");
const categoriesChoices = computed(() => [{label: "All", value: ""}].concat(symbolTypes || []));
let timeout: NodeJS.Timeout | null = null;

const modules = computed<Record<string, { name: string; symbols: ApiSymbol[]; }>>(() => {
return Object.entries(initialModules).reduce((acc, [key, value]) => {
Expand Down Expand Up @@ -74,7 +74,13 @@ const modules = computed<Record<string, { name: string; symbols: ApiSymbol[]; }>
class="transition-opacity duration-500">
<h2 class="font-bold text-xl">{{ module.name }}</h2>

<ApiList class="mt-4" :items="module.symbols"></ApiList>
<div class="bg-gray-100 dark:bg-gray-900 pb-4 p-5 mt-4 mb-10 rounded-sm">
<div class="grid grid-cols-1 sm:grid-cols-3 lg:grid-cols-4 gap-4">
<div v-for="item in module.symbols" :key="item.symbolName">
<ApiAnchor class="w-full px-2 py-1" v-bind="item" theme="list"/>
</div>
</div>
</div>
Comment on lines +77 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Consider adding semantic list roles for better accessibility

The ApiAnchor component is implemented as a semantic link/span with proper attributes, but we should enhance the list structure's accessibility:

  • Add role="list" to the grid container div
  • Add role="listitem" to the v-for div wrapper
-        <div class="grid grid-cols-1 sm:grid-cols-3 lg:grid-cols-4 gap-4">
+        <div class="grid grid-cols-1 sm:grid-cols-3 lg:grid-cols-4 gap-4" role="list">
-          <div v-for="item in module.symbols" :key="item.symbolName">
+          <div v-for="item in module.symbols" :key="item.symbolName" role="listitem">
             <ApiAnchor class="w-full px-2 py-1" v-bind="item" theme="list"/>
           </div>
         </div>
🔗 Analysis chain

LGTM! Clean grid layout implementation with some suggestions.

The grid layout implementation is well-structured with proper responsive design. The use of ApiAnchor with v-bind is efficient.

Consider these minor improvements:

  1. Add role="list" to the grid container for better accessibility
  2. Verify color contrast ratios for dark mode
-          <div class="grid grid-cols-1 sm:grid-cols-3 lg:grid-cols-4 gap-4">
+          <div class="grid grid-cols-1 sm:grid-cols-3 lg:grid-cols-4 gap-4" role="list">

Let's verify if ApiAnchor component handles accessibility properly:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if ApiAnchor component has proper accessibility attributes
ast-grep --pattern 'template {
  $$$
  <$_
    $$$
    role=$_
    $$$
  />
  $$$
}' packages/theme/molecules/api-anchor/ApiAnchor.vue

Length of output: 136


Script:

#!/bin/bash
# Let's check the actual implementation of ApiAnchor component
cat packages/theme/molecules/api-anchor/ApiAnchor.vue

Length of output: 1120

</div>
</template>
</div>
Expand Down
8 changes: 4 additions & 4 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export default defineConfig({
enabled: true,
thresholds: {
autoUpdate: true,
statements: 64.74,
branches: 80.31,
functions: 69.81,
lines: 64.74
statements: 64.61,
branches: 80,
functions: 67.92,
lines: 64.61
},
include: ["**/*.{ts,vue}"],
exclude: [
Expand Down
Loading