-
Notifications
You must be signed in to change notification settings - Fork 6
feat/redesign-and-improvements #20
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
base: main
Are you sure you want to change the base?
feat/redesign-and-improvements #20
Conversation
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
- Moved i18n access to function call time instead of store definition time - Added error handling with a fallback message - Called getInitialMessage() when actually needed in initiateConversation()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
friendly reminder^^:
- indentation
- dark mode default
- something else that i most probably forgot :D
file and sitemap loader still work. confluence not tested... its not configured in the uni deployment ^^
frontend/README.md
Outdated
### UI Customization | ||
- VITE_BOT_NAME = The AI assistant's display name (default: "Knowledge Agent") | ||
- VITE_UI_LOGO_PATH = Path to the main navigation logo (default: "/assets/navigation-logo.svg") | ||
- VITE_UI_THEME_DEFAULT = Default theme when user first visits (default: "light") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: dark might be nice ^^
frontend/libs/shared/settings.ts
Outdated
ui: { | ||
logoPath: "/assets/navigation-logo.svg", | ||
theme: { | ||
default: "light", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thought: i guess here or so
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
- Added features.useMockData to support mock API configuration - Properly handles VITE_USE_MOCK_DATA environment variable - Fixes TypeScript error in document.api.factory.ts
@a-klos FIY: just fixed pipeline and add your suggestions. Not tested it yet on the system. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces environment-based configuration, theming improvements, and enhanced document management for the frontend application. The changes enable better customization through environment variables and provide a more polished user experience.
- Add environment-based settings system with support for bot name, logo path, and theme configuration
- Implement comprehensive light/dark theme system with user preference persistence
- Enhance document management UI with improved interactions and mock API support
Reviewed Changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
tsconfig.base.json | Add path aliases for new shared settings and theme store modules |
tailwind.config.js | Expand theme configuration with comprehensive light/dark color schemes |
ThemeToggle.vue | New component for theme switching with sun/moon icons |
NavigationContainer.vue | Updated to use configurable logo path and include theme toggle |
theme.store.ts | New Pinia store for theme management with localStorage persistence |
settings.ts | New configuration system with environment variable support and fallbacks |
global.css | Add theme-specific chat text styles and avatar improvements |
README.md | Document new environment variables and customization options |
DocumentItem.vue | Enhanced document item UI with confirmation modal and improved status display |
document.api.mock.ts | New mock implementation for document API with realistic data |
document.api.factory.ts | Factory pattern for selecting real vs mock API implementation |
documents.store.ts | Code formatting improvements and better error handling |
App.vue files | Theme store initialization and code formatting updates |
UI_Customization.md | Comprehensive documentation for UI customization features |
options: typeof import.meta.env.VITE_UI_THEME_OPTIONS === "string" && import.meta.env.VITE_UI_THEME_OPTIONS.trim() | ||
? import.meta.env.VITE_UI_THEME_OPTIONS.split(",") | ||
: defaultSettings.ui.theme.options, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The complex conditional logic for parsing VITE_UI_THEME_OPTIONS is hard to read and maintain. Consider extracting this into a separate function like parseThemeOptions()
for better readability.
options: typeof import.meta.env.VITE_UI_THEME_OPTIONS === "string" && import.meta.env.VITE_UI_THEME_OPTIONS.trim() | |
? import.meta.env.VITE_UI_THEME_OPTIONS.split(",") | |
: defaultSettings.ui.theme.options, | |
options: parseThemeOptions(import.meta.env.VITE_UI_THEME_OPTIONS, defaultSettings.ui.theme.options), |
Copilot uses AI. Check for mistakes.
import { ConfluenceConfig, DocumentAPI, SitemapConfig } from "../document.api"; | ||
|
||
export const useDocumentsStore = defineStore('chat', () => { | ||
const uploadedDocuments = ref<UploadedDocument[]>([]); | ||
const allDocuments = ref<DocumentModel[]>(); | ||
const error = ref<ErrorType | null>(null); | ||
const isLoadingConfluence = ref(false); | ||
const isLoadingSitemap = ref(false); | ||
export const useDocumentsStore = defineStore("chat", () => { | ||
const uploadedDocuments = ref<UploadedDocument[]>([]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement references DocumentAPI
directly, but the store should use the factory pattern introduced in document.api.factory.ts
to support mock data functionality. This will cause the mock API feature to not work properly in the documents store.
Copilot uses AI. Check for mistakes.
try { | ||
return t("chat.initialMessage", { bot_name: settings.bot.name }); | ||
} catch (error) { | ||
console.warn("i18n interpolation failed, using fallback", error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The try-catch block uses a generic console.warn message that may not be helpful for debugging i18n interpolation issues. Consider providing more specific error context, such as the actual interpolation parameters that failed.
try { | |
return t("chat.initialMessage", { bot_name: settings.bot.name }); | |
} catch (error) { | |
console.warn("i18n interpolation failed, using fallback", error); | |
const key = "chat.initialMessage"; | |
const params = { bot_name: settings.bot.name }; | |
try { | |
return t(key, params); | |
} catch (error) { | |
console.warn( | |
`i18n interpolation failed for key "${key}" with params ${JSON.stringify(params)}. Using fallback.`, | |
error | |
); |
Copilot uses AI. Check for mistakes.
<!-- Delete confirmation modal --> | ||
<div | ||
v-if="showDeleteModal" | ||
class="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The modal overlay should include keyboard event handling for the Escape key to close the modal, which is a standard accessibility expectation for modal dialogs.
Copilot uses AI. Check for mistakes.
"--base-200-highlight": "#4d4d4d", | ||
|
||
// Fix for chat text color | ||
".chat-text": "#f5f5f5", // Light text color for chat in dark mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Hardcoding CSS classes within the Tailwind theme configuration is unconventional. Consider moving these chat text color overrides to the global.css file or using CSS custom properties for better maintainability.
".chat-text": "#f5f5f5", // Light text color for chat in dark mode |
Copilot uses AI. Check for mistakes.
Overview
This PR introduces several improvements to the frontend application, focusing on customization options and document handling. The main changes include environment-based configuration, enhanced document interactions, and visual refinements.
Changes
Environment-Based Settings Configuration
.env
files using the VITE_ prefixDocument Experience Improvements
Visual Updates
How to Test
.env
file with custom settings:Screenshots
tbd