-
-
Notifications
You must be signed in to change notification settings - Fork 347
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
Several minor improvements to the app #2770
Conversation
Compute the window-in-collection state instead of storing in store Only save current window to collection and don't overwrite other unsaved windows Stop toggling variable dialog when saving to collection
Reviewer's Guide by SourceryThis pull request refactors the handling of window states in the Altair app by computing the window-in-collection state dynamically, removing unnecessary dialog toggles, and ensuring only the current window is saved to the collection. It also introduces a utility function to check for unsaved changes and updates the UI components to reflect these changes. Updated class diagram for WindowService and related componentsclassDiagram
class WindowService {
+setupWindow(windowId: string)
-isWindowInCollection(windowId: string)
-windowHasUnsavedChanges(windowId: string)
}
class WindowSwitcherComponent {
+isWindowInCollection(windowId: string)
+windowHasUnsavedChanges(windowId: string)
}
class HeaderComponent {
+collections: IQueryCollection[]
}
class WindowsEffects {
+windowHasUnsavedChanges(windowState: PerWindowState, collections: IQueryCollection[]): boolean
}
class QueryCollectionEffects {
+LoadCollectionsAction()
}
WindowService --> WindowSwitcherComponent : uses
WindowService --> WindowsEffects : uses
WindowSwitcherComponent --> HeaderComponent : uses
WindowsEffects --> QueryCollectionEffects : uses
note for WindowService "Removed ToggleVariableDialogAction and collection validation logic"
note for WindowSwitcherComponent "Added methods to check window collection state and unsaved changes"
note for HeaderComponent "Added collections input"
note for WindowsEffects "Refactored to use windowHasUnsavedChanges utility"
note for QueryCollectionEffects "Added LoadCollectionsAction"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @imolorhe - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Visit the preview URL for this PR (updated for commit d87425c): https://altair-gql--pr2770-imolorhe-several-imp-tfk3t54o.web.app (expires Thu, 27 Feb 2025 00:21:27 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 02d6323d75a99e532a38922862e269d63351a6cf |
Addressing improvements from user feedback
Fixes
Checks
yarn test-build
Changes proposed in this pull request:
Summary by Sourcery
Improve the window management by dynamically computing the window-in-collection state, preventing unnecessary toggling of the variable dialog, and ensuring only the current window is saved to the collection. Introduce a feature to detect unsaved changes in windows.
New Features:
Bug Fixes:
Enhancements: