-
Notifications
You must be signed in to change notification settings - Fork 29
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
fix: fissure notification options #1325
Conversation
fix: synth portraits
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates several components and static assets. Changes include modifications in computed properties and sorting logic, along with minor syntax adjustments in UI components. Multiple URL references have been updated from the fandom domain to the official wiki domain. Additionally, the notification system now adjusts the notification identifier based on the fissure’s difficulty. A new JavaScript module replaces a deprecated JSON file for trackables data, and the related import in state management is updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant F as Fissure
participant N as Notifier
F->>N: Trigger generateNotifications(fissure)
N->>N: Check if fissure.isHard
alt fissure.isHard true
N->>N: Append ".sp" to notifIdentifier
else
N->>N: Generate notifIdentifier normally
end
N->>N: Create and dispatch notification
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
components/SynthesisImg.jsx (1)
1-46
: Consider using an external configuration file for image mappings.The static image mappings object is quite large. Consider moving it to a separate configuration file to improve maintainability and reduce the component's complexity.
Create a new file
config/synthesis-images.js
:import { optimize, cdn } from '@/services/utilities'; export const synthesisImages = { 'Ancient Disruptor': optimize(cdn('webp/synthesis/ancient_disruptor.webp')), 'Ancient Healer': optimize(cdn('webp/synthesis/ancient_healer.webp')), // ... rest of the mappings };Then import and use it in the component:
import HubImg from '@/components/HubImg.jsx'; import { optimize, cdn } from '@/services/utilities'; +import { synthesisImages } from '@/config/synthesis-images'; const translate = (stub) => (stub ? optimize(cdn(`webp/synthesis/${stub}.webp`)) : null); -const imgs = { - 'Ancient Disruptor': optimize(cdn('webp/synthesis/ancient_disruptor.webp')), - // ... remove the rest -}; +const imgs = synthesisImages;static/json/trackables.js (3)
85-85
: Fix typo in Exilus Adapter text.There's a typo in the text property: "Adapater" should be "Adapter".
- text: 'Exilus Adapater', + text: 'Exilus Adapter',
502-511
: Consider adding validation for fissure combinations.The nested loop generates all possible combinations of tiers and mission types, but some combinations might not be valid in-game. Consider:
- Adding validation to filter out invalid combinations
- Documenting which combinations are actually possible
fissures.tiers.forEach((tier) => { fissures.missionTypes.forEach((missionType) => { + // Skip invalid combinations + if (!isValidFissureCombination(tier, missionType)) return; + const key = `fissures.${tier.value}.${missionType.value}`; eventTypes[key] = { state: false, value: key, text: `${tier.text} ${missionType.text} Fissure`, }; }); }); +/** + * Validates if a fissure combination is possible in-game + * @param {Object} tier - Fissure tier object + * @param {Object} missionType - Mission type object + * @returns {boolean} - Whether the combination is valid + */ +function isValidFissureCombination(tier, missionType) { + // Add validation logic here + return true; +}
1-517
: Great architectural improvement moving from JSON to JS module!This change brings several benefits:
- Better IDE support and type checking
- Dynamic generation of fissure combinations
- Ability to add validation logic and documentation
- More maintainable and extensible structure
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (14)
components/SynthesisImg.jsx
(1 hunks)components/modalDialogs/Filters/ComponentsFilter.jsx
(1 hunks)components/modalDialogs/Filters/FissureFilters.jsx
(1 hunks)components/modalDialogs/Filters/NotificationFilters.jsx
(1 hunks)components/panels/FissuresPanel.vue
(1 hunks)pages/ow/fish/howto/index.vue
(4 hunks)pages/synthesis.vue
(1 hunks)services/Notifier.js
(1 hunks)static/json/fish/deimos.json
(13 hunks)static/json/fish/poe.json
(14 hunks)static/json/fish/vallis.json
(14 hunks)static/json/trackables.js
(1 hunks)static/json/trackables.json
(0 hunks)store/worldstate.js
(1 hunks)
💤 Files with no reviewable changes (1)
- static/json/trackables.json
✅ Files skipped from review due to trivial changes (7)
- pages/synthesis.vue
- pages/ow/fish/howto/index.vue
- components/modalDialogs/Filters/FissureFilters.jsx
- store/worldstate.js
- static/json/fish/vallis.json
- static/json/fish/poe.json
- static/json/fish/deimos.json
🔇 Additional comments (8)
components/SynthesisImg.jsx (2)
65-69
: LGTM! Well-structured data computation.The data function effectively computes the
src
by attempting to translate theimage
prop first, then falling back to the static mapping. This provides good flexibility while maintaining a clean interface.
74-74
: LGTM! Improved data flow.The change from
this.$props.src
tothis.src
is correct as it properly uses the computed source from the component's data, which handles both dynamic and static image paths.components/modalDialogs/Filters/ComponentsFilter.jsx (1)
38-39
: LGTM! Enhanced component sorting.The addition of alphabetical sorting by text property improves the user experience by making components easier to locate.
components/modalDialogs/Filters/NotificationFilters.jsx (1)
56-57
: LGTM! Consistent sorting implementation.The addition of sorting by value property aligns with the sorting in ComponentsFilter, providing a consistent user experience across filters.
components/panels/FissuresPanel.vue (1)
151-155
: LGTM! Improved fissure sorting logic.The sorting logic has been made more explicit and maintainable by:
- Clear primary sort by tier number
- Secondary sort handling storm fissures
- Better readability with separate conditions
services/Notifier.js (1)
316-316
: LGTM! Enhanced fissure notification identification.The notification identifier now properly distinguishes between regular and steel path fissures by appending '.sp' suffix, improving notification categorization and aligning with the PR objective to fix fissure notification options.
static/json/trackables.js (2)
98-369
: LGTM! Well-structured event types.The event types are well-organized with consistent properties and clear naming patterns, particularly for arbitration events.
513-516
: LGTM! Clean export structure.The default export cleanly combines both objects, making them easily importable elsewhere.
## [2.5.3](v2.5.2...v2.5.3) (2025-02-05) ### Bug Fixes * fissure notification options ([#1325](#1325)) ([a1fbadc](a1fbadc))
🎉 This PR is included in version 2.5.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Summary:
also fix: synth portraits
closes #1295
Fixes issue (include link):
Mockups, screenshots, evidence:
Summary by CodeRabbit