Update frontend dependencies & faster navigation #137
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Change CORS origins and URLs from port 3000 to 5173 (Vite default) - Update documentation to reflect new development port - Upgrade e2e testing dependencies: Playwright, TypeScript, Node types - Simplify playwright-bdd configuration and improve test steps - Upgrade UI dependencies for better Vue 3 compatibility - Add Quill mock for balm-ui compatibility - Update import paths and module configurations
- upgraded vue from 3.5.13 to 3.5.17 - upgraded eslint from 9.23.0 to 9.29.0 - upgraded rollup-plugin-visualizer from 5.14.0 to 6.0.3
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…t clicks This was due to a subtle reactivity issue in watchEffect, so all watchEffect usages were refactored to watch
There was a problem hiding this comment.
Pull Request Overview
This PR updates the frontend dependencies with significant changes including switching the development UI server port from 3000 to 5173, upgrading libraries such as Playwright and Vue, and adding a Quill mock for compatibility with balm-ui.
- Updated Vite configuration with a new Quill alias.
- Refactored several Vue composables to adopt improved reactive patterns and updated dependency usage.
- Adjusted API, documentation, E2E tests, and environment files to reflect the new server port and dependency versions.
Reviewed Changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/vite.config.js | Added an alias for Quill pointing to the mock module. |
| ui/src/use.js | Updated reactive logic and navigation functions in composable hooks. |
| ui/src/mocks/quill.js | Added a simple mock to support balm-ui compatibility. |
| ui/src/main.js | Updated import to use the new client entry for @unhead/vue. |
| UI Components (TileViewer.vue, TagEditor.vue, etc.) | Adjusted reactive behavior and event handling to align with dependency updates. |
| api.js, main.go, internal/render/scene.go | Extended API functions for minimal region fetching and range-based queries. |
| E2E tests and configuration files | Updated references to use port 5173 and adjusted test steps accordingly. |
| Documentation files | Updated references to port 5173 throughout. |
Comment on lines
+127
to
+142
| const region = computed(() => { | ||
| if (!id.value) { | ||
| region.value = null; | ||
| return; | ||
| return null; | ||
| } | ||
| if (!valid.value) { | ||
| region.value = { | ||
| return { | ||
| id: id.value, | ||
| loading: true, | ||
| } | ||
| return; | ||
| }; | ||
| } | ||
| if (isValidating.value) return; | ||
| region.value = data.value; | ||
| }) | ||
| if (isValidating.value) { | ||
| return { | ||
| id: id.value, | ||
| loading: true, | ||
| }; | ||
| } |
There was a problem hiding this comment.
[nitpick] The computed property for 'region' now handles various loading and validation states. Please consider adding inline comments to clarify the logic transitions (e.g. handling of invalid, loading, and validated states) for future maintainability.
Copilot uses AI. Check for mistakes.
Comment on lines
+962
to
+977
| return []render.Region{} | ||
| } | ||
|
|
||
| // Parse range in format "start:end" | ||
| parts := strings.Split(rangeStr, ":") | ||
| if len(parts) != 2 { | ||
| return []render.Region{} | ||
| } | ||
|
|
||
| startId, err1 := strconv.Atoi(strings.TrimSpace(parts[0])) | ||
| endId, err2 := strconv.Atoi(strings.TrimSpace(parts[1])) | ||
|
|
||
| if err1 != nil || err2 != nil || startId > endId { | ||
| return []render.Region{} | ||
| } | ||
|
|
There was a problem hiding this comment.
When parsing the id_range parameter, the code currently returns an empty array for malformed inputs. It may be useful to log an error or include additional validation feedback to aid future debugging.
Suggested change
| return []render.Region{} | |
| } | |
| // Parse range in format "start:end" | |
| parts := strings.Split(rangeStr, ":") | |
| if len(parts) != 2 { | |
| return []render.Region{} | |
| } | |
| startId, err1 := strconv.Atoi(strings.TrimSpace(parts[0])) | |
| endId, err2 := strconv.Atoi(strings.TrimSpace(parts[1])) | |
| if err1 != nil || err2 != nil || startId > endId { | |
| return []render.Region{} | |
| } | |
| log.Println("handleRegionsByRange: rangeStr is empty") | |
| return []render.Region{} | |
| } | |
| // Parse range in format "start:end" | |
| parts := strings.Split(rangeStr, ":") | |
| if len(parts) != 2 { | |
| log.Printf("handleRegionsByRange: malformed rangeStr '%s', expected format 'start:end'\n", rangeStr) | |
| return []render.Region{} | |
| } | |
| startId, err1 := strconv.Atoi(strings.TrimSpace(parts[0])) | |
| endId, err2 := strconv.Atoi(strings.TrimSpace(parts[1])) | |
| if err1 != nil || err2 != nil { | |
| log.Printf("handleRegionsByRange: invalid startId or endId in rangeStr '%s' (startId error: %v, endId error: %v)\n", rangeStr, err1, err2) | |
| return []render.Region{} | |
| } | |
| if startId > endId { | |
| log.Printf("handleRegionsByRange: startId (%d) is greater than endId (%d) in rangeStr '%s'\n", startId, endId, rangeStr) | |
| return []render.Region{} | |
| } |
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several updates to improve photo navigation and update frontend and e2e dependencies.
Photo Navigation Enhancements:
RegionMinimalschema and related API changes to support optimized bulk fetching with minimal data fields (idandbounds)Development Configuration Updates:
3000to5173to align with Vite defaults across multiple files (.env.development, [1];README.md, [2];docs/development.md, [3] [4];e2e/playwright.config.ts, [5] [6];e2e/src/fixtures.ts, [7] [8] [9] [10].API Enhancements:
id_rangeandfields) to the API for efficient sequential region fetching and field selection. (api.yaml, [1] [2];internal/openapi/api.gen.go, [3] [4]Sceneto handle minimal region responses, including fetching regions by bounds, image ID, and closest point. (internal/render/scene.go, internal/render/scene.goR412-R460)End-to-End Testing Updates:
e2e/package.json. (e2e/package.json, e2e/package.jsonL22-R27)e2e/playwright.config.ts, [1] [2];e2e/src/steps.ts, [3] [4] [5];e2e/tests/first-run.feature, [6] [7] [8] [9]These changes collectively enhance the application's performance, developer experience, and testing reliability.