feat: add mapillary sequence uploader part-1 #27
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a Mapillary sequence viewing/editing feature (types, store, composable, many components), switches UI from PrimeVue to Naive UI, replaces Prettier JSON with typed config, consolidates ESLint, updates TS/Vite/package configs, and expands auto-imports and global/component type declarations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as MapillaryTable / SequenceForm
participant Composable as useMapillary
participant Store as useMapillaryStore
participant API as /api/mapillary
User->>UI: submit(sequenceId)
UI->>Composable: loadSequence(sequenceId)
Composable->>Store: reset(), setLoading(true), clearError()
Composable->>Composable: validateSequenceId()
alt invalid
Composable->>Store: setError(...)
Composable->>Store: setLoading(false)
else valid
Composable->>API: GET /sequences/:id
API-->>Composable: 200 + images
Composable->>Store: set items, creator, stepper='2'
Composable->>Store: setLoading(false)
end
Note over UI,Store: User interacts (select, edit meta)
UI->>Store: updateItem(...)/set global inputs
User->>UI: preview / submit upload
UI->>Composable: loadSDC(selectedIds) / submitUpload(payload)
Composable->>API: POST /upload or GET /sdc
API-->>Composable: response / error
Composable->>Store: update results or setError(...)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (25)
src/types/mapillary.ts (5)
3-3: Clarify captured_at units (ms vs s).If this mirrors Mapillary Graph API, captured_at is milliseconds since epoch. Encode that in docs or with a branded type to prevent unit bugs.
Example:
-export interface MapillaryImage { - captured_at: number +export interface MapillaryImage { + /** Unix timestamp in milliseconds */ + captured_at: numberOptionally:
type UnixTimeMs = number & { __unit: 'ms' } captured_at: UnixTimeMs
9-11: Make coordinates self-describing.Use a named tuple for readability and to prevent lat/lon swaps.
- coordinates: [number, number] // [longitude, latitude] + coordinates: [lon: number, lat: number] // [longitude, latitude]
14-16: Consider adding smaller thumbs or documenting availability.If consumers may ask for 256/512 sizes, either include optional fields or note that only 1024/original are guaranteed here.
31-36: categories should likely be string[].A single string limits multi-category selection and complicates UI. Switch to string[] (or union with string for backward compat).
-export interface Metadata { +export interface Metadata { title: string description: Description - categories: string + categories: string[] selected: boolean }If existing callers expect string, consider:
categories: string | string[]
40-45: Duplicate id increases drift risk.MapillaryItem.id duplicates image.id. Prefer deriving from image.id or document that they must match.
-export interface MapillaryItem { - image: MapillaryImage - meta: Metadata - index: number - id: string -} +export interface MapillaryItem { + image: MapillaryImage + meta: Metadata + index: number + /** mirrors image.id */ + id: string +}Or drop id and use image.id at call sites.
prettier.config.ts (2)
19-32: Overrides likely redundant; confirm necessity.Prettier auto-selects parsers from extensions. Keep overrides only if you observed mis-parsing or plugin ordering issues.
16-16: htmlWhitespaceSensitivity: 'ignore' can alter Vue template spacing.This broad setting may interact with inline elements (e.g., your JobStatusTag seconds). Consider 'css' (default) unless you need global ignore.
src/components/mapillary/languageOptions.ts (1)
1-4: Add const assertion for immutability.The
languageOptionsarray would benefit from aas constassertion to make it readonly and provide stricter typing. This prevents accidental mutations and gives better type inference.-export const languageOptions = [ - { label: 'English', value: 'en' }, - { label: 'Deutsch', value: 'de' }, -] +export const languageOptions = [ + { label: 'English', value: 'en' }, + { label: 'Deutsch', value: 'de' }, +] as constsrc/components/mapillary/MapillaryBatchPanel.vue (1)
12-14: Consider removing explicit type assertions if events are properly typed.The update handlers use explicit
as stringtype assertions. IfMapillaryInputsproperly types its emitted events, these assertions may be unnecessary and could mask type mismatches. Verify that the event payloads are correctly typed in the component definition.Alternatively, if
MapillaryInputssupports v-model, consider usingv-model:language,v-model:description, andv-model:categoriesfor cleaner two-way binding.src/components/mapillary/MapillarySelectedMessagePanel.vue (1)
19-19: Consider using a constant for layout values.The layout value
'list'is a magic string. If the store defines layout as a union type or enum, consider extracting layout values as constants to prevent typos and improve type safety.src/stores/mapillary.store.ts (2)
6-6: Consider externalizing the default sequence ID.The hardcoded default sequence ID
'tulzukst7vufhdo1e4z60f'appears to be test/example data. Consider moving this to an environment variable or configuration file for easier management.Apply this approach by creating a config file or using an env variable:
// In a config file or using import.meta.env const DEFAULT_SEQUENCE_ID = import.meta.env.VITE_DEFAULT_MAPILLARY_SEQUENCE_ID || ''Then use it in the store:
- const sequenceId = ref<string>('tulzukst7vufhdo1e4z60f') + const sequenceId = ref<string>(DEFAULT_SEQUENCE_ID)
11-14: Layout option labels could be more descriptive.The layout option names "Select" and "Edit" might be clearer as "Grid View" and "List View" to better communicate their purpose to users.
const layoutOptions = ref<{ name: string; value: Layout }[]>([ - { name: 'Select', value: 'grid' }, - { name: 'Edit', value: 'list' }, + { name: 'Grid View', value: 'grid' }, + { name: 'List View', value: 'list' }, ])src/components/mapillary/MapillarySequenceInfo.vue (1)
36-42: Type assertion may be unnecessary.The type assertion
as Layouton line 41 might be redundant ifstore.layoutis already properly typed. TypeScript should infer the type from the store.Verify if the type assertion can be removed:
-@update:model-value="(v) => (store.layout = v as Layout)" +@update:model-value="(v) => (store.layout = v)"If TypeScript complains, the current code is fine.
src/components/mapillary/MapillaryGridView.vue (2)
8-8: Consider responsive grid layout.The grid is fixed at 3 columns (
grid-cols-3), which may not work well on smaller screens or very large displays. Consider using responsive Tailwind classes.-<div class="grid grid-cols-3 gap-4 my-5"> +<div class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4 gap-4 my-5">
24-26: Complex CSS for triangle overlay.The triangle overlay uses border tricks which work but are not immediately obvious. Consider adding a comment to explain the technique.
+<!-- Triangle overlay using border trick for selected indicator --> <div class="w-0 h-0 border-t-[66px] border-r-[66px] border-t-green-500 border-r-transparent opacity-80" ></div>src/composables/useMapillary.ts (4)
4-13: DRY the regex: extract a single SEQ_ID_RE and reuse.Pattern is duplicated here and in validateSequenceId. Extract once to avoid drift.
Apply:
export const useMapillary = () => { const store = useMapillaryStore() + const SEQ_ID_RE = /^[a-zA-Z0-9_-]+$/ // Validation rules (stateless) const validationRules = { sequenceId: [ { required: true, message: 'Please enter a sequence ID' }, { - pattern: /^[a-zA-Z0-9_-]+$/, + pattern: SEQ_ID_RE, message: 'Sequence ID can only contain letters, numbers, underscores, and hyphens', }, ], }
47-63: Avoid per-item pushes; map once and assign for fewer reactive updates.Also, top-level id duplicates image.id; keep one source of truth.
Apply:
- items.forEach((image, index) => { - const date = new Date(image.captured_at).toISOString().split('T')[0] - store.items.push({ - id: image.id, - index: index + 1, - image, - meta: { - title: `Photo from Mapillary ${date} (${image.id}).jpg`, - description: { - language: 'en', - text: '', - }, - categories: '', - selected: false, - }, - }) - }) + const mapped = images.map((image, index) => { + const date = new Date(toMs(image.captured_at)).toISOString().split('T')[0] + return { + // index is useful for UI, id comes from image.id (avoid duplicate top-level id if not needed) + index: index + 1, + image, + meta: { + title: `Photo from Mapillary ${date} (${image.id}).jpg`, + description: { language: 'en', text: '' }, + categories: '', + selected: false, + }, + } + }) + store.items = mapped
65-69: Creator assignment: use the already-sorted first image and guard both fields.Small clarity tweak; no behavior change.
- if (items[0]?.creator?.username) { - store.creatorUsername = items[0].creator.username - store.creatorId = items[0].creator.id - } + const first = images[0] + if (first?.creator?.username && first?.creator?.id) { + store.creatorUsername = first.creator.username + store.creatorId = first.creator.id + }
32-42: Consider request cancellation to avoid race conditions.Rapid calls to loadSequence can interleave; last response should win. Use AbortController or a request token.
If useful, I can provide an AbortController-based patch.
src/components/mapillary/MapillaryTable.vue (2)
10-17: Clear error in store when Message is dismissed.Currently closing the UI message leaves store.error set.
- <Message + <Message v-if="store.error" severity="error" class="mb-4" :closable="true" + @close="store.error = null" > {{ store.error }} </Message>
49-66: Optional: provide stable keys for list/grid items.If MapillaryListView/MapillaryGridView iterate with v-for, ensure they use a stable key like image.id to avoid DOM churn.
src/components/mapillary/MapillaryInputs.vue (1)
43-51: Minor a11y: associate labels with controlsAdd for/id pairs so labels are clickable and announced.
-<label class="block font-medium text-gray-600">Language</label> +<label for="lang" class="block font-medium text-gray-600">Language</label> <Select + input-id="lang" ... /> -<label class="block font-medium text-gray-600">Description</label> +<label for="desc" class="block font-medium text-gray-600">Description</label> <Textarea + id="desc" ... /> -<label class="block text-sm font-medium text-gray-600">Categories</label> +<label for="cats" class="block text-sm font-medium text-gray-600">Categories</label> <Textarea + id="cats" ... />Also applies to: 56-64
src/components/mapillary/MapillaryImageItem.vue (1)
10-12: Tailwind class likely invalid: border-l-10Tailwind doesn’t ship a border-l-10 utility by default. Use arbitrary value or a supported scale.
- class="grid grid-cols-[auto_1fr] gap-4 p-4 border-l-10 border-transparent" + class="grid grid-cols-[auto_1fr] gap-4 p-4 border-l-[10px] border-transparent"eslint.config.ts (2)
25-26: Avoid Prettier/ESLint disagreement on commasIf Prettier’s trailingComma differs, you’ll fight the formatter. Either align Prettier’s trailingComma to always-multiline or drop this rule and let Prettier decide.
- 'comma-dangle': ['error', 'always-multiline'], + // Prefer deferring to Prettier; or ensure prettier.config sets trailingComma: 'all' + // 'comma-dangle': ['error', 'always-multiline'],Also applies to: 47-54
12-20: Ignore patterns: reconsider '*.json'If you want ESLint to validate JSON config (e.g., .eslintrc-auto-import.json), don’t ignore all JSON. Limit the ignore to large data blobs if needed.
- '*.json', + // '*.json', // keep if JSON linting not desired
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.eslintrc-auto-import.json(3 hunks).prettierrc.json(0 hunks)auto-imports.d.ts(6 hunks)components.d.ts(1 hunks)env.d.ts(1 hunks)eslint.config.ts(1 hunks)package.json(2 hunks)prettier.config.ts(1 hunks)src/App.vue(1 hunks)src/components/BotActions.vue(2 hunks)src/components/BotsDataTable.vue(2 hunks)src/components/BotsTable.vue(2 hunks)src/components/Footer.vue(1 hunks)src/components/Header.vue(1 hunks)src/components/JobStatusTag.vue(1 hunks)src/components/mapillary/MapillaryBatchPanel.vue(1 hunks)src/components/mapillary/MapillaryGridView.vue(1 hunks)src/components/mapillary/MapillaryImageItem.vue(1 hunks)src/components/mapillary/MapillaryInputs.vue(1 hunks)src/components/mapillary/MapillaryListView.vue(1 hunks)src/components/mapillary/MapillarySelectedMessagePanel.vue(1 hunks)src/components/mapillary/MapillarySequenceForm.vue(1 hunks)src/components/mapillary/MapillarySequenceInfo.vue(1 hunks)src/components/mapillary/MapillaryTable.vue(1 hunks)src/components/mapillary/languageOptions.ts(1 hunks)src/composables/useBotStatus.spec.ts(1 hunks)src/composables/useMapillary.ts(1 hunks)src/main.ts(1 hunks)src/stores/mapillary.store.ts(1 hunks)src/types/mapillary.ts(1 hunks)tsconfig.app.json(1 hunks)vite.config.ts(2 hunks)
💤 Files with no reviewable changes (1)
- .prettierrc.json
🧰 Additional context used
🧬 Code graph analysis (4)
src/composables/useMapillary.ts (2)
src/stores/mapillary.store.ts (1)
useMapillaryStore(1-111)src/types/mapillary.ts (1)
MapillaryApiResponse(22-24)
src/types/mapillary.ts (1)
auto-imports.d.ts (7)
MapillaryImage(119-119)MapillaryApiResponse(119-119)Metadata(119-119)MetadataKey(119-119)MetadataValue(119-119)MapillaryItem(119-119)Layout(119-119)
auto-imports.d.ts (4)
src/types/mapillary.ts (1)
MetadataKey(37-37)src/components/mapillary/languageOptions.ts (1)
languageOptions(1-4)src/composables/useMapillary.ts (1)
useMapillary(1-82)src/stores/mapillary.store.ts (1)
useMapillaryStore(1-111)
src/stores/mapillary.store.ts (2)
auto-imports.d.ts (4)
MapillaryItem(119-119)Layout(119-119)MetadataKey(119-119)MetadataValue(119-119)src/types/mapillary.ts (4)
MapillaryItem(40-45)Layout(47-47)MetadataKey(37-37)MetadataValue(38-38)
🔇 Additional comments (37)
src/composables/useBotStatus.spec.ts (1)
1-1: LGTM! Import reordering for consistency.The alphabetical reordering of imports has no functional impact.
env.d.ts (1)
7-7: LGTM! Formatting adjustment.End-of-file newline formatting has no functional impact.
vite.config.ts (2)
16-16: LGTM! Tailwind plugin deduplicated.Ensuring
tailwindcss()is invoked only once prevents double initialization.
56-60: LGTM! Mapillary proxy correctly placed.The new
/api/mapillaryproxy is correctly positioned before the catch-all/apiproxy, ensuring specific routes are matched first.src/components/BotActions.vue (1)
17-20: LGTM! Template formatting improved.Multiline formatting enhances readability without changing behavior.
Also applies to: 48-54, 57-62
src/components/JobStatusTag.vue (1)
27-30: Readability tweak LGTM.Splitting v-if and class across lines improves clarity; no functional impact.
src/types/mapillary.ts (1)
47-47: Layout union is clear.Type is concise and future-proof for additional layouts later.
prettier.config.ts (2)
35-35: Config export LGTM.Once the
$schematyping is fixed, default export is fine.
18-18: Verify plugin version and .vue support for import organizationConfirm that
prettier-plugin-organize-importsis at a version compatible with Prettier 3.x and test its behavior on.vuefiles:#!/bin/bash # Show plugin version jq -r '.devDependencies["prettier-plugin-organize-imports"]' package.json # Check import-organizing rules in .vue files rg -n "simple-import-sort|import/order|organize-imports" -g "*.vue"package.json (4)
41-42: Consider the impact of prettier-plugin-organize-imports.The addition of
prettier-plugin-organize-importswill automatically reorganize imports when Prettier runs. This can cause large diffs in existing files if imports aren't already organized according to the plugin's rules.Ensure the team is aware this plugin will reorder imports across the codebase, and consider running Prettier on all files in this PR to avoid surprise changes in future commits.
45-46: TypeScript 5.9.3 uses tilde versioning.TypeScript is pinned with
~5.9.3, which allows only patch updates (5.9.x). This is a good practice for stability but means you'll need to manually update for minor version bumps.
33-33: Confirmed: @vue/eslint-config-prettier@^10.2.0 is valid and up-to-date. No changes required.
15-19: PrimeVue dependencies are up-to-date and secure. All PrimeVue packages are on the latest stable version (4.4.0) and no security advisories were found.tsconfig.app.json (3)
12-17: Strict type checking enabled.Enabling strict mode with
noUnusedLocalsandnoUnusedParametersis excellent for catching potential bugs early. The settingnoPropertyAccessFromIndexSignature: falseprovides flexibility for accessing properties via index signatures.
2-2: Bun and Vue TypeScript config extends.Extending both
@tsconfig/bunand@vue/tsconfig/tsconfig.dom.jsonprovides appropriate defaults for a Bun + Vue environment. Good choice for consistency.
26-29: Alias configuration verified. Imports using@/…match thetsconfig.app.jsonmapping and no improper alias usages were found.src/components/Header.vue (2)
13-15: UI updates look good.The header title simplification from "CuratorBot - Toolforge Jobs" to "Curator" and the padding class change from
md:px-52tolg:px-52align with the broader UI consolidation mentioned in the PR summary.
7-9: No additional error handling required
authStore.checkAuth()already encapsulates its own try–catch (logging and reset) and a finally block, so the call site need not be wrapped again.src/main.ts (1)
1-2: Import reordering aligns with Prettier plugin.Moving the PrimeIcons CSS import after the assets import is likely due to
prettier-plugin-organize-imports. This ensures consistent import ordering across the codebase and may also affect CSS cascade order (assets styles load first).src/components/mapillary/languageOptions.ts (1)
1-4: Limited language support.Currently only English and German are supported. Consider documenting whether additional languages are planned, or if this is intentionally limited to these two languages for the initial Mapillary integration.
src/components/BotsDataTable.vue (1)
22-81: LGTM on formatting changes.The reformatting of Column components improves readability without altering functionality. The multi-line format makes the template structure clearer.
src/components/mapillary/MapillarySequenceForm.vue (1)
25-25: No action needed—.trim()is safe onsequenceId. It’s declared asref<string>and initialized to a string literal ('tulzukst7vufhdo1e4z60f'), so it can’t benullorundefined.src/components/BotsTable.vue (2)
85-92: Formatting change only.Template reformatting with no functional impact.
95-100: Formatting change only.Template reformatting with no functional impact.
src/stores/mapillary.store.ts (1)
1-9: Store structure is well-organized.The store follows Pinia composition API patterns correctly with clear separation of state, actions, and getters. The
$resetfunction properly reinitializes all state to defaults.Also applies to: 16-23, 26-28, 38-62, 66-77, 79-111
src/components/mapillary/MapillarySequenceInfo.vue (2)
22-29: External link security is correct.The Mapillary profile link correctly includes
target="_blank"withrel="noopener noreferrer"to prevent security vulnerabilities.
1-49: Component structure is clean and functional.The component correctly uses the Mapillary store and renders sequence information with appropriate conditional logic.
src/components/mapillary/MapillaryGridView.vue (3)
42-42: Date formatting may need timezone consideration.Using
toLocaleString()without arguments will use the user's browser locale and timezone. This is fine for display purposes, but ensure users understand the timezone context if needed.Consider if timezone display is important for your use case. If so, you can pass options:
new Date(item.image.captured_at).toLocaleString(undefined, { timeZone: 'UTC', timeZoneName: 'short' })
46-54: External link security is correct.The Mapillary image link properly includes
target="_blank"withrel="noopener noreferrer"for security.
1-5: Grid view component is well-structured.The component correctly handles item selection, displays metadata, and provides a clean grid interface for browsing Mapillary images.
Also applies to: 9-23, 27-41, 55-59
.eslintrc-auto-import.json (1)
18-21: ESLint auto-import configuration updated correctly.The new Mapillary-related globals (types, composables, and stores) are properly registered for ESLint auto-import support, consistent with the new feature additions.
Also applies to: 24-26, 63-63, 114-115
src/App.vue (2)
1-3: Ensure ref is in scope.If auto-imports aren’t configured for Vue reactivity APIs, import it.
<script setup lang="ts"> -const activeTab = ref('mapillary') +import { ref } from 'vue' // if not using auto-imports +const activeTab = ref('mapillary') </script>
9-25: Use v-model (v-model:value) for Tabs bindingUsing only :value makes the Tabs controlled but won't update activeTab on click unless the component emits update:value; switch to the library’s v-model syntax (or wire the update event) so UI clicks update the bound state.
- <Tabs - :value="activeTab" - class="mt-4" - > + <Tabs v-model:value="activeTab" class="mt-4"> <TabList> <Tab value="bots">CuratoBot jobs</Tab> <Tab value="mapillary">Mapillary Sequence Viewer</Tab> </TabList>src/composables/useMapillary.ts (1)
45-49: Verify captured_at type and normalize before sorting
In src/composables/useMapillary.ts (lines 45–49), subtracting image.captured_at directly may yield NaN or incorrect order if it’s an ISO string or epoch seconds. Confirm its exact type in MapillaryApiResponse and convert to a millisecond timestamp (e.g. via Date.parse or multiplying seconds) before sorting.src/components/mapillary/MapillaryImageItem.vue (1)
15-21: Display user-friendly one-based index
Ifindexis coming from a zero-basedv-for, update the template to{{ index + 1 }}instead of{{ index }}so users see “1, 2, 3…” rather than “0, 1, 2…”. Confirm in the parent component howindexis passed.auto-imports.d.ts (1)
1-217: Auto-generated file - no manual review required.This file is generated by
unplugin-auto-importand should not be manually edited. The additions correctly reflect the new Mapillary types, composables, and utilities introduced in this PR. Changes will be regenerated automatically when the source files are modified.components.d.ts (1)
1-54: Auto-generated file - no manual review required.This file is generated by
unplugin-vue-componentsand should not be manually edited. The additions correctly register the new Mapillary components and additional PrimeVue components used in this PR for type-safe template usage. Changes will be regenerated automatically when components are added or removed.
| files: ['**/*.{ts,vue}'], | ||
| languageOptions: { | ||
| parser: typescriptParser, | ||
| parser: vueParser, | ||
| parserOptions: { | ||
| project: 'tsconfig.app.json', | ||
| ecmaVersion: 'latest', | ||
| parser: tseslint.parser, | ||
| sourceType: 'module', | ||
| extraFileExtensions, | ||
| projectService: true, | ||
| tsconfigRootDir: import.meta.dirname, | ||
| extraFileExtensions: ['.vue'], | ||
| }, | ||
| globals: { | ||
| ...globals.browser, | ||
| ...autoImportGlobals.globals, | ||
| }, | ||
| }, | ||
| plugins: { |
There was a problem hiding this comment.
Use vue parser only for .vue; ts parser for .ts
Applying vue-eslint-parser to .ts files can cause parse errors and disable TS rules. Split per-file configs.
export default defineConfig(
{
ignores: [
@@
prettierConfig,
- {
- files: ['**/*.{ts,vue}'],
- languageOptions: {
- parser: vueParser,
- parserOptions: {
- ecmaVersion: 'latest',
- parser: tseslint.parser,
- sourceType: 'module',
- projectService: true,
- tsconfigRootDir: import.meta.dirname,
- extraFileExtensions: ['.vue'],
- },
- globals: {
- ...globals.browser,
- ...autoImportGlobals.globals,
- },
- },
- plugins: {
- '@typescript-eslint': tseslint.plugin,
- },
- rules: {
+ // Vue SFCs
+ {
+ files: ['**/*.vue'],
+ languageOptions: {
+ parser: vueParser,
+ parserOptions: {
+ ecmaVersion: 'latest',
+ parser: tseslint.parser,
+ sourceType: 'module',
+ projectService: true,
+ tsconfigRootDir: TS_ROOT,
+ },
+ globals: { ...globals.browser, ...autoImportGlobals.globals },
+ },
+ plugins: { '@typescript-eslint': tseslint.plugin },
+ rules: {
'vue/multi-word-component-names': 'off',
'vue/singleline-html-element-content-newline': 'off',
'vue/html-indent': 'off',
'@typescript-eslint/ban-ts-comment': 'off',
'@typescript-eslint/no-unsafe-assignment': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'comma-dangle': ['error', 'always-multiline'],
@@
'@typescript-eslint/no-unsafe-argument': 'off',
},
},
+ // TypeScript files
+ {
+ files: ['**/*.ts'],
+ languageOptions: {
+ parser: tseslint.parser,
+ parserOptions: {
+ ecmaVersion: 'latest',
+ sourceType: 'module',
+ projectService: true,
+ tsconfigRootDir: TS_ROOT,
+ },
+ globals: { ...globals.browser, ...autoImportGlobals.globals },
+ },
+ plugins: { '@typescript-eslint': tseslint.plugin },
+ },
)Also applies to: 29-37
| const config: Config = { | ||
| $schema: 'https://json.schemastore.org/prettierrc', | ||
| semi: false, |
There was a problem hiding this comment.
Type error: $schema is not in Prettier’s Config type.
This will fail TS checking. Use satisfies or remove $schema.
-const config: Config = {
- $schema: 'https://json.schemastore.org/prettierrc',
+const config = {
+ $schema: 'https://json.schemastore.org/prettierrc',
semi: false,
singleQuote: true,
...
-}
+} satisfies ConfigOr drop $schema if you don’t need editor schema hints.
📝 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 config: Config = { | |
| $schema: 'https://json.schemastore.org/prettierrc', | |
| semi: false, | |
| import type { Config } from 'prettier'; | |
| const config = { | |
| $schema: 'https://json.schemastore.org/prettierrc', | |
| semi: false, | |
| singleQuote: true, | |
| // ... | |
| } satisfies Config; | |
| export default config; |
🤖 Prompt for AI Agents
In prettier.config.ts around lines 3 to 5, the config object includes a $schema
property which is not part of Prettier's Config type and causes a TypeScript
error; fix it by either removing the $schema entry if you don't need editor JSON
schema hints, or keep $schema but change the declaration so the object either
uses the `satisfies Config` operator (e.g. const config = { ... } satisfies
Config) or cast the object to unknown as Config, ensuring TypeScript no longer
reports `$schema` as an excess property while preserving the schema hint.
src/components/BotsDataTable.vue
Outdated
| :value="botsStore.bots" | ||
| :loading="isLoading" | ||
| stripedRows | ||
| striped-rows |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the correct prop name for PrimeVue DataTable.
The prop was changed from stripedRows to striped-rows. PrimeVue typically uses camelCase for prop names in script but kebab-case is valid in templates. Verify that striped-rows is correctly recognized by the DataTable component in your PrimeVue version.
🌐 Web query:
PrimeVue DataTable striped rows prop name
💡 Result:
The DataTable prop is named stripedRows (use as a boolean attribute, e.g. ). [1][2]
Sources:
[1] PrimeVue DataTable docs (Tailwind example) — stripedRows usage.
[2] PrimeVue DataTable docs (primevue.org) — DataTable API and examples.
Replace striped-rows with stripedRows
The DataTable prop is named stripedRows in PrimeVue and must be used as a boolean attribute (e.g. <DataTable :value="items" stripedRows>). (v3.primevue.org)
🤖 Prompt for AI Agents
In src/components/BotsDataTable.vue around line 18, the template uses the
incorrect prop name `striped-rows`; replace it with the PrimeVue boolean prop
`stripedRows` (i.e., use the camelCase attribute `stripedRows` on the DataTable
component) so it is recognized correctly; ensure it is used as a boolean
attribute (no value binding needed) or as a bound boolean (e.g.,
:stripedRows="true") consistent with other props in the file.
| :disabled="store.selectedCount === 0" | ||
| :label=" | ||
| store.showSelectedOnly && store.selectedCount > 0 | ||
| ? 'Show all items' | ||
| : 'Show only selected' | ||
| " | ||
| icon="pi pi-filter" | ||
| class="rounded-md!" | ||
| @click="() => (store.showSelectedOnly = !store.showSelectedOnly)" | ||
| /> |
There was a problem hiding this comment.
Avoid inline arrow fn and fix class typo.
Inline arrow allocates each render; class "rounded-md!" looks accidental.
- <Button
+ <Button
:disabled="store.selectedCount === 0"
:label="
store.showSelectedOnly && store.selectedCount > 0
? 'Show all items'
: 'Show only selected'
"
icon="pi pi-filter"
- class="rounded-md!"
- @click="() => (store.showSelectedOnly = !store.showSelectedOnly)"
+ class="rounded-md"
+ @click="store.showSelectedOnly = !store.showSelectedOnly"
/>📝 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.
| :disabled="store.selectedCount === 0" | |
| :label=" | |
| store.showSelectedOnly && store.selectedCount > 0 | |
| ? 'Show all items' | |
| : 'Show only selected' | |
| " | |
| icon="pi pi-filter" | |
| class="rounded-md!" | |
| @click="() => (store.showSelectedOnly = !store.showSelectedOnly)" | |
| /> | |
| <Button | |
| :disabled="store.selectedCount === 0" | |
| :label=" | |
| store.showSelectedOnly && store.selectedCount > 0 | |
| ? 'Show all items' | |
| : 'Show only selected' | |
| " | |
| icon="pi pi-filter" | |
| class="rounded-md" | |
| @click="store.showSelectedOnly = !store.showSelectedOnly" | |
| /> |
🤖 Prompt for AI Agents
In src/components/mapillary/MapillaryTable.vue around lines 37 to 46, the button
uses an inline arrow function and has a stray "!" in the class name; replace the
inline arrow expression (@click="() => (store.showSelectedOnly =
!store.showSelectedOnly)") with a component method or a referenced handler
(e.g., @click="toggleShowSelectedOnly") defined in methods or setup so it isn't
reallocated on every render, and remove the accidental exclamation from the
class (change "rounded-md!" to "rounded-md").
| const validateSequenceId = (sequenceId: string): boolean => { | ||
| // Clear previous error | ||
| store.error = null | ||
|
|
||
| const trimmed = sequenceId.trim() | ||
| if (!trimmed) { | ||
| store.error = 'Please enter a sequence ID' | ||
| return false | ||
| } |
There was a problem hiding this comment.
Input not normalized: trim and persist the cleaned value.
validateSequenceId trims but loadSequence later uses the untrimmed store value. Persist the trimmed value before validating.
Apply:
- const validateSequenceId = (sequenceId: string): boolean => {
+ const validateSequenceId = (sequenceId: string): boolean => {
// Clear previous error
store.error = null
- const trimmed = sequenceId.trim()
+ const trimmed = sequenceId.trim()
if (!trimmed) {
store.error = 'Please enter a sequence ID'
return false
}
- const pattern = /^[a-zA-Z0-9_-]+$/
- if (!pattern.test(trimmed)) {
+ if (!SEQ_ID_RE.test(trimmed)) {
store.error = 'Sequence ID can only contain letters, numbers, underscores, and hyphens'
return false
}
return true
}And in loadSequence:
- const sequenceId = store.sequenceId
- if (!validateSequenceId(sequenceId)) return
+ const sequenceId = store.sequenceId.trim()
+ store.sequenceId = sequenceId
+ if (!validateSequenceId(sequenceId)) returnCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/composables/useMapillary.ts around lines 15 to 23, validateSequenceId
trims the input but does not persist the cleaned value back to the store and
loadSequence later uses the original untrimmed value; update validateSequenceId
to assign the trimmed value into store.sequenceId before performing validation,
and update loadSequence to use a trimmed version of store.sequenceId (e.g., read
const seq = store.sequenceId.trim() and use seq) so the normalized value is
consistently used throughout.
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
src/components/mapillary/MapillarySequenceForm.vue (1)
7-10: Form submission prevention now handled correctly.The previous review comment requested preventing default form submission. This has been addressed by implementing
handleSubmitwithe.preventDefault(). The current approach works correctly.src/stores/mapillary.store.ts (1)
32-38: Non-null assertion remains unsafe.The code still uses non-null assertions
items.value[id]!(lines 33-34), which will throw if the ID doesn't exist. This is the same issue raised in previous reviews but with Record access instead offind().Apply this diff to add safety:
const updateItem = (id: string, key: MetadataKey, value: MetadataValue) => { - const meta = items.value[id]!.meta - items.value[id]!.meta = { - ...meta, - [key]: value, - } + const item = items.value[id] + if (!item) { + console.error(`Item with id ${id} not found`) + return + } + item.meta = { + ...item.meta, + [key]: value, + } }src/components/mapillary/MapillaryInputs.vue (1)
34-35: Verify languageOptions is auto-imported and non-empty.The component references
languageOptionswithout an explicit import (line 34) and assumes it's non-empty with the non-null assertionlanguageOptions[0]!(line 35).Based on the AI summary,
languageOptionsshould exist atsrc/components/mapillary/languageOptions.ts. Verify it's properly exported and configured for auto-import:#!/bin/bash # Verify languageOptions exists and is exported fd -g '**/languageOptions.ts' -x cat {} # Check if it's configured for auto-import rg -nC3 'languageOptions' vite.config.ts auto-imports.d.ts
🧹 Nitpick comments (14)
src/App.vue (1)
2-6: Consider removing or populating the empty theme overrides.The
themeOverridesobject contains only an emptycommonproperty with a commentedprimaryColor. If theme customization isn't needed yet, remove the object; otherwise, uncomment and configure the values.Apply this diff to remove the unused override:
-const themeOverrides: GlobalThemeOverrides = { - common: { - // primaryColor: '#0E47CB', - }, -} - const activeTab = ref('mapillary')Or uncomment to apply the theme:
const themeOverrides: GlobalThemeOverrides = { common: { - // primaryColor: '#0E47CB', + primaryColor: '#0E47CB', }, }src/components/mapillary/MapillaryUpload.vue (2)
9-9: Replaceanywith proper type for type safety.The render function parameter uses
any, which eliminates TypeScript's type checking benefits. Define a proper interface for the row data or infer it fromstore.displayedItems.Apply this diff to add a type:
+interface DisplayedItem { + meta: { + title: string + } +} + const columns = [ { title: '#', key: 'index' }, { title: 'Title', key: 'title', - render: (row: any) => `File:${row.meta.title}`, + render: (row: DisplayedItem) => `File:${row.meta.title}`, }, ]
5-5: Use Naive UI’s index column instead ofkey
Rows instore.displayedItemsdon’t include anindexfield, so{ title: '#', key: 'index' }won’t render numbers. Replace it with Naive UI’s built-in index column ({ title: '#', type: 'index' }) or add arenderfunction to displayrowIndex + 1.src/components/wikidata/StatementsList.vue (1)
2-3: Consider removing explicit imports if auto-import is configured.The PR introduces auto-import configuration via
unplugin-vue-components. IfPropertyLabelandStatementItemare automatically imported elsewhere in the codebase, these explicit imports may be redundant.Apply this diff if auto-import handles these components:
-import PropertyLabel from '@/components/wikidata/PropertyLabel.vue' -import StatementItem from '@/components/wikidata/StatementItem.vue' - defineProps<{ statements: Statement[] }>()src/components/wikidata/useWikidata.ts (2)
1-12: Consider scalability for property mappings.The hardcoded property mapping covers 11 common Wikidata properties. As the feature evolves, this may require frequent updates. Consider:
- Documenting the criteria for which properties are included
- Adding a mechanism to dynamically fetch labels from Wikidata API if the mapping grows significantly
- Centralizing these mappings in a configuration file if they're shared across components
20-28: LGTM! Consider logging for missing mappings.The implementation is clean and safe with nullish coalescing. For debugging during development, consider logging when a property or entity isn't found:
export const useWikidata = () => { - const getPropertyLabel = (property: string) => properties[property] ?? '' - const getEntityLabel = (entity: string) => entities[entity] ?? '' + const getPropertyLabel = (property: string) => { + if (!properties[property]) { + console.warn(`Unknown Wikidata property: ${property}`) + } + return properties[property] ?? '' + } + const getEntityLabel = (entity: string) => { + if (!entities[entity]) { + console.warn(`Unknown Wikidata entity: ${entity}`) + } + return entities[entity] ?? '' + } return { getPropertyLabel, getEntityLabel, } }This helps identify missing mappings early.
src/components/mapillary/MapillarySequenceEdit.vue (3)
22-37: Remove or implement the commented toggle button.The commented filter button suggests incomplete functionality. Either:
- Implement the feature if it's planned
- Remove the code if it's not needed
64-71: Consider MapillaryListView API design.Passing a single-item array
[item]toMapillaryListViewin a loop seems unusual. If MapillaryListView is designed to render multiple items, consider:
- Rendering all
store.displayedItemsat once:<MapillaryListView :items="store.displayedItems" />- Or renaming to
MapillaryListItemif it's meant for single itemsThis pattern may cause performance issues with large lists due to component overhead.
38-49: Centralize stepper states
Define a single enum or constant object for the various stepper values (currently'2','3','4','5') and replace all hardcoded strings (store.stepper = '…') with these named constants to improve maintainability.src/stores/mapillary.store.ts (2)
46-57: Consider validation for language codes.The
setGlobalLanguagefunction accepts any string. Consider validating that the language code follows a standard format (e.g., ISO 639-1 like 'en', 'es') to prevent invalid values.
86-86: Clarify or remove displayRows.The
displayRowsgetter returns a constant value (5) regardless of stepper state. This appears to be a placeholder or the ternary is unnecessary. Consider:
- Removing it if unused
- Documenting its purpose if it's a feature in progress
- Making the value configurable if it represents pagination
src/components/mapillary/MapillaryPreview.vue (1)
41-43: Add error handling for SDC loading.The
loadSDC()call inonMountedlacks visible error handling. If loading fails, users won't see any feedback. Consider adding try-catch or checking ifloadSDChandles errors internally.onMounted(async () => { - await loadSDC() + try { + await loadSDC() + } catch (error) { + console.error('Failed to load SDC data:', error) + // Optionally show user-facing error message + } })src/components/mapillary/MapillaryGridView.vue (1)
7-28: Mixed icon library usage detected.The component uses PrimeIcons classes (
pi pi-checkon line 27,pi pi-external-linkon line 53) while the project is migrating to Naive UI and using@vicons/ionicons5. Consider replacing these with consistent icon components from the new icon library.Example for check icon:
-<i class="pi pi-check text-white absolute top-2 left-2 text-xl!"></i> +<n-icon class="text-white absolute top-2 left-2" size="20"> + <CheckmarkOutline /> +</n-icon>Import the icon:
<script setup lang="ts"> +import { CheckmarkOutline, OpenOutline } from '@vicons/ionicons5' defineProps<{ items: MapillaryItem[] }>()src/components/Header.vue (1)
4-6: Add error handling for auth check.The async
checkAuth()call lacks error handling. If authentication fails, users won't see any feedback.onMounted(async () => { - await authStore.checkAuth() + try { + await authStore.checkAuth() + } catch (error) { + console.error('Auth check failed:', error) + } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.eslintrc-auto-import.json(3 hunks)auto-imports.d.ts(7 hunks)components.d.ts(1 hunks)package.json(1 hunks)postcss.config.ts(1 hunks)src/App.vue(1 hunks)src/assets/main.css(1 hunks)src/components/BotActions.vue(1 hunks)src/components/BotsDataTable.vue(1 hunks)src/components/BotsTable.vue(2 hunks)src/components/Footer.vue(1 hunks)src/components/Header.vue(1 hunks)src/components/JobStatusTag.vue(1 hunks)src/components/mapillary/MapillaryBatchPanel.vue(1 hunks)src/components/mapillary/MapillaryGridView.vue(1 hunks)src/components/mapillary/MapillaryImageItem.vue(1 hunks)src/components/mapillary/MapillaryInputs.vue(1 hunks)src/components/mapillary/MapillaryPreview.vue(1 hunks)src/components/mapillary/MapillarySelectedMessagePanel.vue(1 hunks)src/components/mapillary/MapillarySequenceEdit.vue(1 hunks)src/components/mapillary/MapillarySequenceForm.vue(1 hunks)src/components/mapillary/MapillarySequenceInfo.vue(1 hunks)src/components/mapillary/MapillarySequenceSelect.vue(1 hunks)src/components/mapillary/MapillaryTable.vue(1 hunks)src/components/mapillary/MapillaryUpload.vue(1 hunks)src/components/mapillary/languageOptions.ts(1 hunks)src/components/wikidata/PropertyLabel.vue(1 hunks)src/components/wikidata/StatementItem.vue(1 hunks)src/components/wikidata/StatementsList.vue(1 hunks)src/components/wikidata/VR.vue(1 hunks)src/components/wikidata/ValueRenderer.vue(1 hunks)src/components/wikidata/useWikidata.ts(1 hunks)src/composables/useMapillary.ts(1 hunks)src/main.ts(0 hunks)src/stores/mapillary.store.ts(1 hunks)src/types/mapillary.ts(1 hunks)src/types/wikidata.ts(1 hunks)tsconfig.app.json(1 hunks)vite.config.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/main.ts
✅ Files skipped from review due to trivial changes (1)
- postcss.config.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/mapillary/languageOptions.ts
- src/composables/useMapillary.ts
- src/components/BotsTable.vue
- src/components/mapillary/MapillaryBatchPanel.vue
- src/components/JobStatusTag.vue
- src/components/mapillary/MapillaryTable.vue
🧰 Additional context used
🧬 Code graph analysis (4)
src/types/mapillary.ts (1)
src/types/wikidata.ts (1)
Statement(113-119)
src/stores/mapillary.store.ts (1)
src/types/mapillary.ts (4)
MapillaryItem(41-47)Layout(49-49)MetadataKey(38-38)MetadataValue(39-39)
src/types/wikidata.ts (1)
auto-imports.d.ts (22)
WikibaseEntityType(126-126)DataValueEntityId(126-126)DataValueMonolingualText(126-126)DataValueQuantity(126-126)DataValueTime(126-126)DataValueGlobeCoordinate(126-126)StringDataValue(126-126)EntityIdDataValue(126-126)MonolingualTextDataValue(126-126)QuantityDataValue(126-126)TimeDataValue(126-126)GlobeCoordinateDataValue(126-126)UrlDataValue(126-126)DataValue(126-126)SnakType(126-126)ValueSnak(126-126)SomeValueSnak(126-126)NoValueSnak(126-126)Snak(126-126)Reference(126-126)Rank(126-126)Statement(126-126)
auto-imports.d.ts (4)
src/components/mapillary/languageOptions.ts (1)
languageOptions(1-4)src/composables/useMapillary.ts (1)
useMapillary(1-185)src/stores/mapillary.store.ts (1)
useMapillaryStore(1-122)src/components/wikidata/useWikidata.ts (1)
useWikidata(20-28)
🔇 Additional comments (34)
src/types/mapillary.ts (1)
1-49: Types look solidContracts align with usage in store/composable (coords order noted, captured_at numeric). LGTM.
.eslintrc-auto-import.json (1)
8-62: Note: Tabler icons referencedGlobals reference Tabler icons (IconEye, IconFilter, IconSearch, IconUpload). Ensure @vicons/tabler is installed (see package.json comment) to avoid missing modules during typecheck/build.
auto-imports.d.ts (1)
10-14: Verify Tabler icon dependencyThese globals import from '@vicons/tabler'. Add the package if not yet present to prevent module resolution errors.
package.json (1)
21-51: Add missing @vicons/tabler dependency
Auto-generated imports in auto-imports.d.ts reference Tabler icons but the package isn’t listed. Add to devDependencies:"devDependencies": { + "@vicons/tabler": "^0.13.0",Align the version with your existing
@vicons/ionicons5entry.src/components/mapillary/MapillarySelectedMessagePanel.vue (1)
1-27: LGTM!The component correctly displays the selection count and disables the "Start editing" button when no items are selected. The state transitions through
store.stepperare appropriate for the workflow.src/components/wikidata/VR.vue (1)
1-27: LGTM!The component correctly handles all three snak types (
value,novalue,somevalue) with appropriate rendering logic. The conditional structure is clear and the ValueRenderer delegation is appropriate.src/components/wikidata/StatementItem.vue (1)
1-31: LGTM!The component correctly renders the main snak and qualifiers with proper key bindings and nested structure. The layout and conditional rendering logic are appropriate.
src/components/mapillary/MapillarySequenceInfo.vue (1)
1-33: LGTM!The component correctly displays sequence metadata with proper conditional rendering for the creator section. The external link includes appropriate security attributes (
rel="noopener noreferrer") to prevent tabnabbing attacks.src/components/wikidata/StatementsList.vue (1)
7-23: LGTM!The column definitions using
h()render functions are appropriate for programmatic component creation. The use of Tailwind's arbitrary variant syntax (text-right!) is valid and the overall structure is clean.src/components/wikidata/PropertyLabel.vue (2)
1-4: LGTM!Clean composition API setup with proper type safety.
6-15: LGTM!Proper external link security attributes and clear display of both label and property ID for reference.
src/components/mapillary/MapillarySequenceEdit.vue (2)
1-6: LGTM!Clean setup with proper separation between store state and actions.
8-20: LGTM!Clear user feedback about which items are being displayed.
src/stores/mapillary.store.ts (4)
1-26: LGTM!State declarations are well-typed and organized. Using
Record<string, MapillaryItem>for items improves lookup performance compared to array-based storage.
40-44: LGTM!Safe iteration and update pattern without non-null assertions.
59-70: LGTM!Comprehensive reset function that properly reinitializes all state properties.
88-122: LGTM!Clear and comprehensive store interface with well-organized exports.
vite.config.ts (4)
8-9: LGTM!Correct setup of Tailwind CSS v4 and Naive UI resolver. Based on learnings, Tailwind v4 uses the first-party Vite plugin which provides better performance.
Also applies to: 16-16
17-36: LGTM!Proper Naive UI auto-import configuration with type-only imports correctly marked.
37-43: LGTM!Components auto-import properly configured for Naive UI with TypeScript declarations enabled.
67-72: Verify callback rewrite usage and backend expectations. No references to/callback/wikimediaor/auth/callbackwere found in the frontend code; ensure the backend is configured to handle/auth/callbackand that your OAuth flow actually uses/callback/wikimediaas the public callback URL.src/components/mapillary/MapillaryPreview.vue (3)
1-6: LGTM!Clean component setup with proper separation of store state and composable logic.
46-82: LGTM!Clear preview UI with proper button states and pagination configuration.
8-39: No HTML injection risk in wikitext rendering
Vue’sh('pre', …, wikitext(row))escapes text by default, so any HTML indescription.textorcategoriesis shown verbatim inside<pre>, preventing XSS.src/components/mapillary/MapillaryGridView.vue (3)
1-5: LGTM!Clean component setup with proper prop typing.
29-33: LGTM!Proper image component usage with descriptive alt text.
34-57: LGTM!Card footer provides clear information with proper external link handling and locale-aware time formatting.
src/components/Header.vue (2)
10-28: LGTM!Clean migration to Naive UI components with proper layout and responsive styling.
29-39: LGTM!Login button properly handles loading states and provides clear user feedback.
src/components/mapillary/MapillarySequenceSelect.vue (1)
1-81: LGTM! Clean data table implementation.The component properly configures the Naive UI data table with:
- Selection controls
- Interactive image thumbnails that toggle selection
- Well-structured metadata display with external links
- Proper security attributes (noopener noreferrer) on external links
- Integration with the Mapillary store
tsconfig.app.json (1)
1-36: LGTM! Well-configured TypeScript settings.The configuration properly:
- Extends from Bun and Vue base configs
- Enables strict type checking with appropriate compiler flags
- Includes all necessary type declaration files
- Sets up path aliases correctly
- Includes types for Vite, Bun, Node, and Naive UI
src/components/mapillary/MapillaryImageItem.vue (1)
1-67: Well-structured component with proper store integration.The component effectively:
- Uses a clean grid layout for visual hierarchy
- Properly binds selection state and title to the store
- Implements lazy loading for images
- Includes secure external links with
noopener noreferrer- Integrates MapillaryInputs with conditional enable/disable based on global state
Also applies to: 70-131
src/types/wikidata.ts (1)
1-119: LGTM! Comprehensive Wikidata type definitions.The type definitions accurately represent the Wikidata data model with:
- Proper union types for DataValue variants
- Correct discriminated unions using the
typefield- Appropriate string types for Wikidata-specific formats (amounts, times, URLs)
- Complete coverage of Snak, Reference, and Statement structures
components.d.ts (1)
1-56: Auto-generated file - no review needed.This file is automatically generated by
unplugin-vue-componentsand should not be manually edited. The component declarations correctly reflect the addition of Mapillary and Wikidata components, and the migration from PrimeVue to Naive UI.
| .row-selected-global td { | ||
| @apply bg-green-50!; | ||
| } |
There was a problem hiding this comment.
Fix Tailwind important syntax; wrap in utilities layer
Use the important modifier as a prefix, not a suffix. Prefer registering custom utilities in the utilities layer for correct order.
Apply:
-/* Add global utility classes here */
-.row-selected-global td {
- @apply bg-green-50!;
-}
+/* Add global utility classes here */
+@layer utilities {
+ .row-selected-global td {
+ @apply !bg-green-50;
+ }
+}Based on learnings
🤖 Prompt for AI Agents
In src/assets/main.css around lines 4 to 6 the Tailwind important modifier is
used as a suffix and the custom utility is not registered in the utilities
layer; change the rule to register it inside an @layer utilities block and use
the prefix form of the important modifier: move the .row-selected-global td rule
into @layer utilities and apply the utility with the ! prefix (i.e. @apply
!bg-green-50) so Tailwind emits the important variant in the correct order.
| <n-button | ||
| v-if="bot.status.action.type === 'start'" | ||
| :severity="bot.status.action.severity" | ||
| :label="bot.status.action.label" | ||
| icon="pi pi-play" | ||
| :type="bot.status.action.severity === 'danger' ? 'error' : 'default'" | ||
| size="small" | ||
| :loading="jobsStore.starting[bot.type]" | ||
| :disabled="isLoading" | ||
| @click="onStart(bot.type)" | ||
| /> | ||
| <Button | ||
| > | ||
| <template #icon> | ||
| <i class="pi pi-play"></i> | ||
| </template> | ||
| {{ bot.status.action.label }} | ||
| </n-button> | ||
| <n-button | ||
| v-else-if="bot.status.action.type === 'terminate' || bot.status.action.type === 'stop'" | ||
| :severity="bot.status.action.severity" | ||
| :type="bot.status.action.severity === 'danger' ? 'error' : 'default'" | ||
| size="small" | ||
| icon="pi pi-stop" | ||
| :label="bot.status.action.label" | ||
| :loading="jobsStore.deleting[bot.type]" | ||
| :disabled="isLoading" | ||
| @click="onStop(bot.type)" | ||
| /> | ||
| <Tag | ||
| > | ||
| <template #icon> | ||
| <i class="pi pi-stop"></i> | ||
| </template> | ||
| {{ bot.status.action.label }} | ||
| </n-button> | ||
| <n-tag | ||
| v-else-if="bot.status.action.type === 'pending'" | ||
| :severity="bot.status.action.severity" | ||
| :type="bot.status.action.severity === 'danger' ? 'error' : 'default'" | ||
| size="small" | ||
| icon="pi pi-spin pi-spinner" | ||
| :value="bot.status.action.label" | ||
| /> | ||
| <Tag v-else severity="danger" size="small" icon="pi pi-question" value="Unknown" /> | ||
| > | ||
| <template #icon> | ||
| <i class="pi pi-spin pi-spinner"></i> | ||
| </template> | ||
| {{ bot.status.action.label }} | ||
| </n-tag> | ||
| <n-tag | ||
| v-else | ||
| type="error" | ||
| size="small" | ||
| > | ||
| <template #icon> | ||
| <i class="pi pi-question"></i> | ||
| </template> | ||
| Unknown | ||
| </n-tag> | ||
| </div> | ||
| <div v-else-if="authStore.isAuthenticated && !authStore.isAuthorized"> | ||
| <Tag severity="danger" size="small" icon="pi pi-lock" value="Not authorized" /> | ||
| <n-tag | ||
| type="error" | ||
| size="small" | ||
| > | ||
| <template #icon> | ||
| <i class="pi pi-lock"></i> | ||
| </template> | ||
| Not authorized | ||
| </n-tag> |
There was a problem hiding this comment.
Replace PrimeIcons; map severities comprehensively
- PrimeIcons classes remain (play/stop/spinner/question/lock) but primeicons is no longer installed → broken icons.
- Map only 'danger'→'error'; other severities (warning/success/info) lose semantic coloring.
Icons with vicons + Naive UI:
- <n-button
+ <n-button
v-if="bot.status.action.type === 'start'"
- :type="bot.status.action.severity === 'danger' ? 'error' : 'default'"
+ :type="severityMap[bot.status.action.severity] ?? 'default'"
size="small"
:loading="jobsStore.starting[bot.type]"
:disabled="isLoading"
@click="onStart(bot.type)"
>
- <template #icon>
- <i class="pi pi-play"></i>
- </template>
+ <template #icon><n-icon><IconPlayerPlay /></n-icon></template>
{{ bot.status.action.label }}
</n-button>
- <n-button
+ <n-button
v-else-if="bot.status.action.type === 'terminate' || bot.status.action.type === 'stop'"
- :type="bot.status.action.severity === 'danger' ? 'error' : 'default'"
+ :type="severityMap[bot.status.action.severity] ?? 'default'"
size="small"
:loading="jobsStore.deleting[bot.type]"
:disabled="isLoading"
@click="onStop(bot.type)"
>
- <template #icon>
- <i class="pi pi-stop"></i>
- </template>
+ <template #icon><n-icon><IconPlayerStop /></n-icon></template>
{{ bot.status.action.label }}
</n-button>
- <n-tag
+ <n-tag
v-else-if="bot.status.action.type === 'pending'"
- :type="bot.status.action.severity === 'danger' ? 'error' : 'default'"
+ :type="severityMap[bot.status.action.severity] ?? 'default'"
size="small"
>
- <template #icon>
- <i class="pi pi-spin pi-spinner"></i>
- </template>
+ <template #icon><n-icon class="animate-spin"><IconLoader2 /></n-icon></template>
{{ bot.status.action.label }}
</n-tag>
- <n-tag
+ <n-tag
v-else
type="error"
size="small"
>
- <template #icon>
- <i class="pi pi-question"></i>
- </template>
+ <template #icon><n-icon><IconQuestionMark /></n-icon></template>
Unknown
</n-tag>
...
- <div v-else-if="authStore.isAuthenticated && !authStore.isAuthorized">
- <n-tag
+ <div v-else-if="authStore.isAuthenticated && !authStore.isAuthorized">
+ <n-tag
type="error"
size="small"
>
- <template #icon>
- <i class="pi pi-lock"></i>
- </template>
+ <template #icon><n-icon><IconLock /></n-icon></template>
Not authorized
</n-tag>Add in script:
// <script setup>
import { IconPlayerPlay, IconPlayerStop, IconLoader2, IconQuestionMark, IconLock } from '@vicons/tabler'
// Map severities to Naive types
const severityMap: Record<string, 'default' | 'primary' | 'info' | 'success' | 'warning' | 'error'> = {
danger: 'error',
warning: 'warning',
success: 'success',
info: 'primary',
}Note: ensure @vicons/tabler is installed (see package.json comment).
| const columns = [ | ||
| { | ||
| title: 'Type', | ||
| key: 'type', | ||
| render: (row: unknown) => { | ||
| const typedRow = row as BotRow | ||
| if (typedRow?.type) { | ||
| return h('span', { class: 'font-bold' }, typedRow.type) | ||
| } | ||
| return h('span', { class: 'text-gray-400 italic' }, 'Unknown') | ||
| }, | ||
| }, | ||
| { | ||
| title: 'Status', | ||
| key: 'status', | ||
| render: (row: unknown) => { | ||
| const typedRow = row as BotRow | ||
| return h(JobStatusTag, { status: typedRow.status }) | ||
| }, | ||
| }, | ||
| { | ||
| title: 'Job', | ||
| key: 'jobName', | ||
| render: (row: unknown) => { | ||
| const typedRow = row as BotRow | ||
| if (typedRow?.jobName) { | ||
| return h('span', { class: 'font-mono text-sm' }, typedRow.jobName) | ||
| } | ||
| return null | ||
| }, | ||
| }, | ||
| { | ||
| title: 'Command', | ||
| key: 'command', | ||
| render: (row: unknown) => { | ||
| const typedRow = row as BotRow | ||
| return h('code', { class: 'text-sm' }, | ||
| typedRow.command + (typedRow.args ? ' ' + typedRow.args.join(' ') : ''), | ||
| ) | ||
| }, | ||
| }, | ||
| ] |
There was a problem hiding this comment.
Make columns reactive to auth; add typing
columns.push(...) runs once at setup. If auth changes (login/logout), the Actions column won’t update. Also, render functions use unknown, losing type-safety.
Apply:
-const columns = [
+const baseColumns: DataTableColumns<Bot> = [
{
title: 'Type',
key: 'type',
- render: (row: unknown) => {
- const typedRow = row as BotRow
+ render: (row: Bot) => {
+ const typedRow = row as unknown as BotRow
if (typedRow?.type) {
return h('span', { class: 'font-bold' }, typedRow.type)
}
return h('span', { class: 'text-gray-400 italic' }, 'Unknown')
},
},
{
title: 'Status',
key: 'status',
- render: (row: unknown) => {
- const typedRow = row as BotRow
+ render: (row: Bot) => {
+ const typedRow = row as unknown as BotRow
return h(JobStatusTag, { status: typedRow.status })
},
},
{
title: 'Job',
key: 'jobName',
- render: (row: unknown) => {
- const typedRow = row as BotRow
+ render: (row: Bot) => {
+ const typedRow = row as unknown as BotRow
if (typedRow?.jobName) {
return h('span', { class: 'font-mono text-sm' }, typedRow.jobName)
}
return null
},
},
{
title: 'Command',
key: 'command',
- render: (row: unknown) => {
- const typedRow = row as BotRow
+ render: (row: Bot) => {
+ const typedRow = row as unknown as BotRow
return h('code', { class: 'text-sm' },
typedRow.command + (typedRow.args ? ' ' + typedRow.args.join(' ') : ''),
)
},
},
]
-if (authStore.isAuthorized) {
- columns.push({
- title: 'Actions',
- key: 'actions',
- render: (row: unknown) => {
- const typedRow = row as BotRow
- return h(BotActions, {
- bot: typedRow,
- onStart: props.handleStartJob,
- onStop: props.handleDeleteJob,
- })
- },
- })
-}
+const actionsColumn: DataTableColumns<Bot>[number] = {
+ title: 'Actions',
+ key: 'actions',
+ render: (row: Bot) => {
+ const typedRow = row as unknown as BotRow
+ return h(BotActions, {
+ bot: row as Bot, // pass the full Bot object
+ onStart: props.handleStartJob,
+ onStop: props.handleDeleteJob,
+ })
+ },
+}
+
+const columns = computed<DataTableColumns<Bot>>(() =>
+ authStore.isAuthorized ? [...baseColumns, actionsColumn] : baseColumns
+)This keeps Actions in sync with auth and improves type safety.
Also applies to: 64-77
🤖 Prompt for AI Agents
In src/components/BotsDataTable.vue around lines 21-62 (and similarly for
64-77), the columns array is created once with render functions typed as unknown
and thus won’t react to auth changes; change it to a reactive/computed columns
value that depends on the auth state (e.g., computed(() =>
baseColumns.concat(auth.value ? actionsColumn : []))) so the Actions column is
added/removed when login/logout happens, and replace render signatures to use
the concrete BotRow type instead of unknown (or use the table column generic) so
render functions receive a properly typed row; ensure any concatenation of args
handles undefined safely and that the computed is used where columns are
consumed.
| <n-layout-footer | ||
| class="fixed bottom-0 flex justify-between items-center border-l-0! border-r-0! border-b-0! md:px-52! w-full" | ||
| > |
There was a problem hiding this comment.
Tailwind important modifier used incorrectly
Suffix form border-l-0! / md:px-52! won’t work. Use ! as a prefix.
- <n-layout-footer
- class="fixed bottom-0 flex justify-between items-center border-l-0! border-r-0! border-b-0! md:px-52! w-full"
- >
+ <n-layout-footer
+ class="fixed bottom-0 flex justify-between items-center !border-l-0 !border-r-0 !border-b-0 md:!px-52 w-full"
+ >Based on learnings
📝 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.
| <n-layout-footer | |
| class="fixed bottom-0 flex justify-between items-center border-l-0! border-r-0! border-b-0! md:px-52! w-full" | |
| > | |
| <n-layout-footer | |
| class="fixed bottom-0 flex justify-between items-center !border-l-0 !border-r-0 !border-b-0 md:!px-52 w-full" | |
| > |
🤖 Prompt for AI Agents
In src/components/Footer.vue around lines 10 to 12, the Tailwind important
modifier is used as a suffix (e.g., border-l-0! and md:px-52!) which is invalid;
update those utilities to use the prefix form so they become !border-l-0 and
!md:px-52 (and any other suffixed utilities in the same element) to ensure the
important modifier is applied correctly across responsive variants.
| <span class="pi pi-github"></span> | ||
| | ||
| <template |
There was a problem hiding this comment.
Replace PrimeIcons with Naive UI + vicons
PrimeVue/primeicons were removed; <span class="pi ..."> will render nothing. Use <n-icon> with vicons.
Example changes:
- <span class="pi pi-github"></span>
+ <n-icon><LogoGithub /></n-icon>Add import in script:
// in <script setup>
import { LogoGithub } from '@vicons/ionicons5'Alternatively use Tabler’s BrandGithub if you add @vicons/tabler.
🤖 Prompt for AI Agents
In src/components/Footer.vue around lines 15 to 17, the code uses a PrimeIcons
span (<span class="pi pi-github">) which no longer exists; replace it with Naive
UI's n-icon plus a vicons icon. Import the desired vicon in the <script setup>
(for example LogoGithub from @vicons/ionicons5 or BrandGithub from
@vicons/tabler), then replace the span with <n-icon :component="ImportedIcon" />
(or equivalent binding) and remove the old pi class usage so the GitHub icon
renders correctly with Naive UI.
| <span class="text-gray-500">Compass Angle</span> | ||
| <div>{{ image.compass_angle.toFixed(1) }}°</div> |
There was a problem hiding this comment.
Add null check for compass_angle to prevent runtime error.
The previous issue was that a v-if="image.compass_angle" check would hide 0° values. The v-if was removed, but now there's no null/undefined check at all. Calling .toFixed(1) on null or undefined will throw a runtime error.
Apply this diff to add a proper null check:
- <div>
+ <div v-if="image.compass_angle != null">
<span class="text-gray-500">Compass Angle</span>
<div>{{ image.compass_angle.toFixed(1) }}°</div>
</div>This checks for both null and undefined while allowing 0 to be displayed correctly.
📝 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.
| <span class="text-gray-500">Compass Angle</span> | |
| <div>{{ image.compass_angle.toFixed(1) }}°</div> | |
| <div v-if="image.compass_angle != null"> | |
| <span class="text-gray-500">Compass Angle</span> | |
| <div>{{ image.compass_angle.toFixed(1) }}°</div> | |
| </div> |
🤖 Prompt for AI Agents
In src/components/mapillary/MapillaryImageItem.vue around lines 68-69, calling
image.compass_angle.toFixed(1) can throw if compass_angle is null or undefined;
change the rendering to first check that image.compass_angle !== null &&
image.compass_angle !== undefined and only then call .toFixed(1) and append the
degree symbol, otherwise render a fallback (e.g., "—" or "N/A") so that 0 is
still displayed correctly.
| <span>{{ value.value.amount }} {{ value.value.unit }}</span> | ||
| </template> | ||
| <template v-else-if="value.type === 'time'"> | ||
| <span>{{ new Date(value.value.time.replace(/^\+0+/, '')).toISOString().split('T')[0] }}</span> |
There was a problem hiding this comment.
Fix time parsing regex to handle all Wikidata time formats.
The regex /^\+0+/ only matches when there's at least one zero after the plus sign. Wikidata times like "+2013-01-01T00:00:00Z" (without leading zeros) won't match, leaving the "+" in the string, which may cause new Date() to fail or behave unexpectedly.
Apply this diff to correctly strip the leading plus sign and any zeros:
- <span>{{ new Date(value.value.time.replace(/^\+0+/, '')).toISOString().split('T')[0] }}</span>
+ <span>{{ new Date(value.value.time.replace(/^\+0*/, '')).toISOString().split('T')[0] }}</span>The regex /^\+0*/ matches "+" followed by zero or more "0"s, correctly handling both "+2013..." and "+00002013...".
🤖 Prompt for AI Agents
In src/components/wikidata/ValueRenderer.vue around line 24, the time parsing
regex only strips a '+' when it is followed by one or more zeros (`/^\+0+/`), so
strings like "+2013-01-01T00:00:00Z" keep the '+' and can break Date parsing;
update the replace call to use `/^\+0*/` to remove a leading '+' and any number
of zeros (including none), so replace(/^\+0*/, '') before constructing the Date.
1 participant
No description provided.