feat:(zmskvr-299): removal of calendar from workstation #937
Conversation
Deleted Whitespace
Deleted whitespace at End of line 38
WalkthroughThis pull request introduces conditional checks and new methods in the workstation JavaScript view, ensuring that page-specific actions such as loading client data and calendar views occur only when appropriate. Additionally, the backend captures the current date and obtains extra workstation information for response rendering. Several Twig templates have been updated to remove redundant waiting time details, refactor traffic light indicators, and adjust layout dimensions by removing the calendar component and modifying grid sizes. Changes
Sequence Diagram(s)sequenceDiagram
participant Process as Queue/Appointment Process
participant View as Workstation View
Process->>View: Execute onQueueProcess / onReserveProcess / onChangeProcess / onSaveProcess
View->>View: Load appointment form
alt Page is "workstation"
View->>View: Call loadClientNext(showLoader, loadProcess)
end
sequenceDiagram
participant Caller as UI Component
participant View as Workstation View
Caller->>View: Call loadCalendar(showLoader)
alt Page is "counter"
View->>View: Instantiate and return CalendarView
else
View-->>Caller: Return without CalendarView
end
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
Note 🎁 Summarized by CodeRabbit FreeYour organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
|
matthilo96
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
zmsadmin/templates/block/process/next.twig (1)
87-140: Consider consolidating CSS stylesWhile the styling implementation works well, consider moving these styles to an external CSS file instead of embedding them in the template. This would improve maintainability and ensure consistency with other traffic light indicators in the application (like the one in queue/info.twig).
This would also align with best practices for separating content from presentation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
zmsadmin/js/page/workstation/index.js(7 hunks)zmsadmin/src/Zmsadmin/WorkstationProcess.php(3 hunks)zmsadmin/templates/block/process/footer.twig(1 hunks)zmsadmin/templates/block/process/next.twig(1 hunks)zmsadmin/templates/block/queue/info.twig(1 hunks)zmsadmin/templates/page/workstation.twig(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`**/*.{js,jsx,ts,tsx}`: Flag any usage of console.log() as i...
**/*.{js,jsx,ts,tsx}: Flag any usage of console.log() as it should be replaced with proper logging:
- For development: Use proper debug tools or logging libraries
- For production: Remove console.log() statements or use structured logging
- For errors: Use error tracking services (e.g., Sentry)
- For debugging: Consider using debug libraries that can be enabled/disabled
Example replacement:
// Instead of: console.log('User data:', userData); // Use: logger.debug('Processing user data', { userData }); // or for development only: if (process.env.NODE_ENV === 'development') { debug('User data:', userData); }
zmsadmin/js/page/workstation/index.js
`**/*.php`: Flag any usage of error_log() as it should be re...
**/*.php: Flag any usage of error_log() as it should be replaced with proper logging mechanisms:
- For error handling: Use a proper logging framework with error levels (PSR-3 LoggerInterface)
- For debugging: Use a dedicated debug logger or remove debug statements
- For CLI output: Use a CLI output handler or symfony/console
- For application events: Use structured logging with proper log levels
Example replacement:
// Instead of: error_log("Import failed - " . $e->getMessage()); // Use: $logger->error("Import failed", ['error' => $e->getMessage()]);
zmsadmin/src/Zmsadmin/WorkstationProcess.php
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: call-php-build-images / build (zmsadmin, 8.0)
- GitHub Check: call-php-build-images / build (zmsstatistic, 8.0)
- GitHub Check: call-php-build-images / build (zmscalldisplay, 8.0)
- GitHub Check: call-php-build-images / build (zmsticketprinter, 8.0)
🔇 Additional comments (17)
zmsadmin/templates/block/process/footer.twig (1)
1-3: Layout simplified to support calendar removalThe template has been streamlined to only display the current time, which aligns with the PR objective of removing the calendar from the workstation view.
zmsadmin/src/Zmsadmin/WorkstationProcess.php (4)
10-10: LGTM: Necessary import addedThe DateTime class is appropriately imported to support date formatting functionality.
29-29: LGTM: Current date capture implementedCurrent date is properly captured and formatted for consistent display.
38-38: LGTM: Helper method utilized properlyThe workstation info is appropriately gathered using the helper method with the right parameters.
43-45: LGTM: Template variables correctly updatedThe template now receives the necessary data to support the modified workstation view.
zmsadmin/templates/page/workstation.twig (2)
19-19: LGTM: Height adjustment supports layout changesReduced minimum height from 850px to 775px - this adjustment supports the PR objective of reducing distance between queue table and mask.
23-25: LGTM: Width adjustment implements UI changesChanged grid item width from six-twelfths to nine-twelfths, properly implementing the PR objective to widen the mask after calendar removal.
zmsadmin/js/page/workstation/index.js (7)
206-207: LGTM: Page-specific client data loading addedAppropriately added conditional check to load client data only when on the workstation page.
239-240: LGTM: Consistent pattern for workstation pageFollows the same pattern of conditionally loading client data for the workstation page.
265-266: LGTM: Consistent implementation continuesMaintains consistency in the approach to workstation-specific functionality.
293-294: LGTM: Client data loading after saveClient data is appropriately refreshed after process save operation.
353-354: LGTM: Consistent pattern applicationContinues the pattern of workstation-specific client data loading.
539-540: LGTM: Partial reload optimizationOptimizes reloads by only updating the client data when on the workstation page.
551-565: LGTM: Calendar removal implementedThis change implements the core PR objective by only loading the calendar component on the counter page, effectively removing it from the workstation view.
zmsadmin/templates/block/process/next.twig (2)
25-26: Clean button element closureThe button element is now properly closed, improving HTML structure. This is a good change for code correctness.
32-85: Great addition of waiting time and traffic light indicatorsThe footer section with waiting customer metrics provides valuable information for operators. The traffic light system offers an intuitive visual indicator of queue status, and the detailed waiting time statistics help manage customer expectations.
Some considerations:
- The conditional logic for displaying waiting times is well-implemented
- The structure using list elements with cell divisions creates a clear layout
zmsadmin/templates/block/queue/info.twig (1)
138-151: Improved traffic light implementationThe refactored traffic light implementation uses a cleaner approach with a single list item and conditional classes. This is more maintainable than separate elements for each condition.
The waiting customers variable assignment at the beginning improves readability by separating data processing from presentation.
| <style> | ||
|
|
||
| .traffic-light { | ||
| display: flex; | ||
| width: 100%; | ||
| height: 100%; | ||
| } | ||
|
|
||
| .wartende { | ||
| display: flex; | ||
| margin-left: 10px; | ||
| } | ||
|
|
||
| .cell.right { | ||
| display: flex; | ||
| align-items: center; /* Vertically align the items in the center */ | ||
| } | ||
|
|
||
| .waiting-count { | ||
| font-size: 16px; /* Style for the number */ | ||
| font-weight: bold; | ||
| margin-right: 10px; | ||
| } | ||
|
|
||
| .green { | ||
| background-color: rgba(0, 255, 0, 0.5); /* Green */ | ||
| } | ||
|
|
||
| .yellow { | ||
| background-color: rgba(255, 255, 0, 0.5); /* Yellow */ | ||
| } | ||
|
|
||
| .orange { | ||
| background-color: rgba(255, 127, 0, 0.5); /* Orange */ | ||
| } | ||
|
|
||
| .red { | ||
| background-color: rgba(255, 0, 0, 0.5); /* Red */ | ||
| } | ||
|
|
||
| </style> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
CSS styling duplication across templates
The CSS styles for traffic lights are duplicated between this file and process/next.twig. This creates a maintenance challenge if styles need to be updated.
Consider extracting these styles to a common CSS file to maintain consistency and improve maintainability. This would ensure that traffic light indicators look the same throughout the application and make style updates easier.
- <style>
-
- .traffic-light {
- display: flex;
- width: 100%;
- height: 100%;
- }
-
- .wartende {
- display: flex;
- margin-left: 10px;
- }
-
- .cell.right {
- display: flex;
- align-items: center; /* Vertically align the items in the center */
- }
-
- .waiting-count {
- font-size: 16px; /* Style for the number */
- font-weight: bold;
- margin-right: 10px;
- }
-
- .green {
- background-color: rgba(0, 255, 0, 0.5); /* Green */
- }
-
- .yellow {
- background-color: rgba(255, 255, 0, 0.5); /* Yellow */
- }
-
- .orange {
- background-color: rgba(255, 127, 0, 0.5); /* Orange */
- }
-
- .red {
- background-color: rgba(255, 0, 0, 0.5); /* Red */
- }
-
- </style>Add these styles to a common CSS file (e.g., traffic-light.css) and include it in your application's main stylesheet.
📝 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.
| <style> | |
| .traffic-light { | |
| display: flex; | |
| width: 100%; | |
| height: 100%; | |
| } | |
| .wartende { | |
| display: flex; | |
| margin-left: 10px; | |
| } | |
| .cell.right { | |
| display: flex; | |
| align-items: center; /* Vertically align the items in the center */ | |
| } | |
| .waiting-count { | |
| font-size: 16px; /* Style for the number */ | |
| font-weight: bold; | |
| margin-right: 10px; | |
| } | |
| .green { | |
| background-color: rgba(0, 255, 0, 0.5); /* Green */ | |
| } | |
| .yellow { | |
| background-color: rgba(255, 255, 0, 0.5); /* Yellow */ | |
| } | |
| .orange { | |
| background-color: rgba(255, 127, 0, 0.5); /* Orange */ | |
| } | |
| .red { | |
| background-color: rgba(255, 0, 0, 0.5); /* Red */ | |
| } | |
| </style> |
matthilo96
deleted the
feature-zmskvr-299-removal-of-calendar-from-workstation
branch
Pull Request Checklist (Feature Branch to
next):nextBranch in meinen Feature-Branch gemergt.Beschreibung:
Summary by CodeRabbit
New Features
Bug Fixes
Style