Improvement/metric refactor#147
Conversation
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request makes extensive changes to the dropdown components across the frontend. The modifications include replacing the old Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DropdownSelect as Dropdown Select
participant DropdownItem as Dropdown Item
participant Parent as Parent Component
User->>DropdownSelect: Click to open dropdown
DropdownSelect->>DropdownItem: Render inline options
DropdownItem-->>DropdownSelect: Emit selected value
DropdownSelect->>Parent: Propagate update via v-model
Possibly related PRs
Suggested labels
🪧 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.
Pull Request Overview
This PR modernizes the metric selection by replacing the old metricOptions‑based dropdown with a new activities dropdown that leverages a platforms config file. Key changes include:
- Creating a new platforms config file and removing the outdated metricOptions list.
- Renaming the metrics dropdown to activities dropdown and refactoring related components.
- Updating all downstream usages (projects, contributors, collections) to use the new dropdown components.
Reviewed Changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/app/components/uikit/dropdown/dropdown.vue | Refactored to wrap content in lfx-popover and remove legacy pv-select functionality. |
| frontend/app/components/uikit/dropdown/dropdown-separator.vue | Added a new, simple separator component for dropdown menus. |
| frontend/app/components/uikit/dropdown/dropdown-selector.vue | Introduced a new selector component for dropdown trigger presentation. |
| frontend/app/components/uikit/dropdown/dropdown-select.vue | Created to manage dropdown selection using recursive slot scanning for dropdown items. |
| frontend/app/components/uikit/dropdown/dropdown-item.vue | Updated for injection of the selection value and to support the new dropdown behavior. |
| frontend/app/components/modules/project/components/shared/header/date-range-picker.vue | Migrated from lfx-popover to the new lfx-dropdown-select and adjusted trigger logic. |
| frontend/app/components/modules/project/components/contributors/*.vue | Switched from metric dropdown to the activities dropdown to reflect the renamed functionality. |
| frontend/app/components/modules/collection/views/collection-list.vue | Replaced the legacy dropdown with the new lfx-dropdown-select, updating sort options accordingly. |
| frontend/app/components/modules/collection/components/list/filters/* | Removed deprecated dropdown filter components in favor of the new approach. |
Files not reviewed (1)
- frontend/app/components/uikit/dropdown/dropdown.scss: Language not supported
Comments suppressed due to low confidence (4)
frontend/app/components/uikit/dropdown/dropdown.vue:15
- Consider adding a null-check for the 'popover' ref (e.g., using optional chaining) before invoking closePopover() to prevent potential runtime errors.
@click="popover.closePopover()"
frontend/app/components/uikit/dropdown/dropdown-select.vue:55
- [nitpick] Consider adding an explicit return type for 'findDropdownItems' to improve type clarity and maintainability.
const findDropdownItems = (nodes: VNode[], result: VNode[] = []) => {
frontend/app/components/uikit/dropdown/dropdown-item.vue:35
- Consider providing a fallback value or null check for 'selectedValue' to ensure it is defined and prevent potential runtime errors.
const selectedValue = inject<ReturnType<typeof computed<string>>>('selectedValue');
frontend/app/components/modules/project/components/shared/header/date-range-picker.vue:14
- Consider using optional chaining when accessing 'selectedOption.value' (e.g., selectedOption?.value) to avoid potential runtime errors if 'selectedOption' is undefined.
<span v-if="selectedOption.value !== 'custom'">{{ selectedOption?.label }}</span>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (19)
frontend/app/config/platforms/configs/reddit.platform.ts (1)
1-19: Clear implementation of Reddit platform configurationThe configuration is well-structured and follows the same pattern as other platform configs.
Consider changing "Posted a post" to something less redundant like "Created a post" or "Submitted a post" for better readability:
key: ActivityTypes.POST.valueOf(), - label: 'Posted a post' + label: 'Created a post'frontend/app/config/platforms/configs/hackernews.platform.ts (1)
1-19: Hacker News platform configuration implemented correctlyThe configuration follows the established pattern for platform configs and includes appropriate activity types.
Similar to the Reddit configuration, consider changing "Posted a post" to something less redundant like "Created a post" or "Submitted a story" for better clarity:
key: ActivityTypes.POST.valueOf(), - label: 'Posted a post' + label: 'Submitted a story'frontend/app/config/platforms/configs/discord.platform.ts (1)
5-7: Consider adding a comment to explain the use of valueOf()The use of
valueOf()on enum values is not a typical pattern in TypeScript. If there's a specific reason for this approach rather than direct access (e.g.,ActivityPlatforms.DISCORD), consider adding a comment to explain why.frontend/app/config/platforms/configs/git.platform.ts (1)
38-41: Consider rewording "Committed a commit"The label "Committed a commit" sounds redundant. Consider simplifying to "Committed" or rephrasing to clarify the specific meaning if it's distinct from the general act of committing.
- label: 'Committed a commit' + label: 'Committed'frontend/app/config/platforms/configs/gerrit.platform.ts (1)
1-47: Platform configuration looks good with minor duplication issueThe Gerrit platform configuration is well-structured and follows the
PlatformConfiginterface correctly. However, there's a duplication in the activity types:The
CHANGESET_NEW(lines 10-13) andCHANGESET_CREATED(lines 14-17) activity types have identical labels "Created a changeset". This duplication could cause confusion when displaying these activities to users.Consider keeping only one of these activity types or differentiating their labels if they represent distinct actions.
frontend/app/components/uikit/popover/popover.scss (1)
13-21: Clean implementation of modal functionality!The new
.is-modalclass elegantly transforms the popover into a full-screen modal with a semi-transparent background, proper positioning, and centered content. This enhancement adds versatility to the existing popover component.Consider adding a brief comment above this class to document its purpose and usage, which would help other developers understand when to use the modal functionality versus standard popover behavior.
frontend/app/components/uikit/dropdown/dropdown-item.vue (1)
1-46: Well-structured dropdown item component with good TypeScript integrationThis new component is well-designed with clear separation of template, props, and logic. The use of TypeScript for props adds type safety, and the implementation of dependency injection creates a cohesive dropdown system.
Consider adding a safeguard for when the component is used outside of a dropdown context by checking if
selectedValueis undefined during injection:-const selectedValue = inject<ReturnType<typeof computed<string>>>('selectedValue'); +const selectedValue = inject<ReturnType<typeof computed<string>>>('selectedValue', null); + +if (!selectedValue) { + console.warn('LfxDropdownItem should be used within an LfxDropdownSelect component'); +}frontend/app/components/uikit/dropdown/dropdown-select.vue (2)
54-68: Consider optimizing the recursive search functionThe recursive function to find dropdown items is functional but might become inefficient with deeply nested slot content. Consider adding a performance optimization like memoization if this becomes a performance bottleneck in the future.
75-78: Consider enhancing the default label for unmatched selectionsWhen no matching dropdown item is found, the code falls back to using the raw value as both value and label. For better user experience, consider implementing a more descriptive fallback or provide a way to customize this fallback behavior.
- return selected?.props || { value: value.value, label: value.value } + return selected?.props || { value: value.value, label: value.value, description: 'Custom selection' }frontend/app/components/modules/project/components/contributors/fragments/activities-dropdown.vue (2)
89-92: Optimize platform icon lookupThe
getIconfunction performs a linear search through platforms on every render, which could be inefficient for large platform lists. Consider using a computed property or memoization to optimize this lookup.-const getIcon = (platform: string) => { - const platformArr = Object.values(platforms); - return (platform !== 'all' ? platformArr.find((p) => p.key === platform)?.image : '') -}; +const platformsArray = computed(() => Object.values(platforms)); +const getIcon = (platform: string) => { + return (platform !== 'all' ? platformsArray.value.find((p) => p.key === platform)?.image : '') +};
91-91: Handle potential undefined image sourcesThe current implementation returns an empty string for the 'all' platform, but may return undefined for other platforms that aren't found. Consider providing a default image or fallback for these cases.
- return (platform !== 'all' ? platformArr.find((p) => p.key === platform)?.image : '') + return (platform !== 'all' ? platformArr.find((p) => p.key === platform)?.image || 'default-icon-path.svg' : '')frontend/app/components/modules/project/components/shared/header/date-range-picker.vue (4)
23-41: Check for data-driven approach or central config.
Defining dropdown items inline is simpler, but if new date range options are frequently added, consider reading from a shared config or store to avoid duplication across components.
43-63: Use descriptive placeholders for large sets of date options.
With many date options, consider grouping them under more descriptive headings or submenus for clarity. This can improve usability for larger date ranges.
86-93: Minor duplication of "Custom" label.
Line 92 repeats the label “Custom,” which is also specified via thelabelprop in line 88. While not functionally incorrect, consider removing the extra text to avoid confusion for screen readers or potential i18n issues.- @click="isCustomSelectorOpen = true" -> - Custom + @click="isCustomSelectorOpen = true"
138-143: Validate date range changes.
ChangingselectedDateRangetriggers an immediate update ofselectedTimeRangeKey,startDate, andendDate. If these transitions need to be batched or validated, consider adding error handling or additional checks to avoid partially updated states.frontend/app/components/uikit/dropdown/dropdown.scss (4)
1-7: Container Styles and Commented Code Cleanup
The base container styles for.c-dropdownare well defined using utility classes. However, the commented-out@screen smblock (lines 4–6) adds clutter. Consider removing this commented code if it’s no longer needed, or include a clarifying comment if it's meant for future reference.
8-26: Dropdown Item Styling Is Solid
The styling for the dropdown items using the nested&__itemselector is clear and effective. The hover state and the selected state (.is-selected) are implemented correctly. In future refactors, you might consider consolidating multiple utility classes if your team’s style guidelines favor brevity, but this is optional.
36-51: Dropdown Selector Styling – Consider Consolidation of Utility Classes
The styling for&__selectoris robust, with clear handling for hover states and size/filled modifiers. Note that there are two consecutive@applydirectives (lines 37–38) which could potentially be combined into a single statement for improved readability. This is a minor nitpick, and current functionality is unaffected.
54-60:is-openModifier Structuring
The.is-openblock effectively targets the.c-dropdown__selectorto apply the desired style when open. As a potential improvement, consider if applying an additional modifier class directly to.c-dropdown(rather than nesting inside.is-open) might simplify the CSS cascade and enhance maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (41)
frontend/app/components/modules/collection/components/details/filters.vue(2 hunks)frontend/app/components/modules/collection/components/list/filters/collection-filter-industry.vue(0 hunks)frontend/app/components/modules/collection/components/list/filters/collection-filter-stack.vue(0 hunks)frontend/app/components/modules/collection/views/collection-list.vue(2 hunks)frontend/app/components/modules/project/components/contributors/config/metrics.ts(0 hunks)frontend/app/components/modules/project/components/contributors/contributor-dependency.vue(2 hunks)frontend/app/components/modules/project/components/contributors/contributors-leaderboard.vue(2 hunks)frontend/app/components/modules/project/components/contributors/fragments/activities-dropdown.vue(1 hunks)frontend/app/components/modules/project/components/contributors/fragments/metric-dropdown.vue(0 hunks)frontend/app/components/modules/project/components/contributors/geographical-distribution.vue(2 hunks)frontend/app/components/modules/project/components/contributors/organization-dependency.vue(2 hunks)frontend/app/components/modules/project/components/contributors/organizations-leaderboard.vue(2 hunks)frontend/app/components/modules/project/components/shared/header/date-range-picker.vue(3 hunks)frontend/app/components/uikit/dropdown/dropdown-group-title.vue(1 hunks)frontend/app/components/uikit/dropdown/dropdown-item.vue(1 hunks)frontend/app/components/uikit/dropdown/dropdown-select.vue(1 hunks)frontend/app/components/uikit/dropdown/dropdown-selector.vue(1 hunks)frontend/app/components/uikit/dropdown/dropdown-separator.vue(1 hunks)frontend/app/components/uikit/dropdown/dropdown.scss(1 hunks)frontend/app/components/uikit/dropdown/dropdown.vue(1 hunks)frontend/app/components/uikit/popover/popover.scss(1 hunks)frontend/app/components/uikit/popover/popover.vue(6 hunks)frontend/app/config/platforms/configs/confluence.platform.ts(1 hunks)frontend/app/config/platforms/configs/devto.platform.ts(1 hunks)frontend/app/config/platforms/configs/discord.platform.ts(1 hunks)frontend/app/config/platforms/configs/discourse.platform.ts(1 hunks)frontend/app/config/platforms/configs/gerrit.platform.ts(1 hunks)frontend/app/config/platforms/configs/git.platform.ts(1 hunks)frontend/app/config/platforms/configs/github.platform.ts(1 hunks)frontend/app/config/platforms/configs/gitlab.platform.ts(1 hunks)frontend/app/config/platforms/configs/groupsio.platform.ts(1 hunks)frontend/app/config/platforms/configs/hackernews.platform.ts(1 hunks)frontend/app/config/platforms/configs/jira.platform.ts(1 hunks)frontend/app/config/platforms/configs/linkedin.platform.ts(1 hunks)frontend/app/config/platforms/configs/reddit.platform.ts(1 hunks)frontend/app/config/platforms/configs/slack.platform.ts(1 hunks)frontend/app/config/platforms/configs/stackoverflow.platform.ts(1 hunks)frontend/app/config/platforms/configs/twitter.platform.ts(1 hunks)frontend/app/config/platforms/index.ts(1 hunks)frontend/types/shared/activity-platforms.ts(1 hunks)frontend/types/shared/platforms.types.ts(1 hunks)
💤 Files with no reviewable changes (4)
- frontend/app/components/modules/collection/components/list/filters/collection-filter-stack.vue
- frontend/app/components/modules/project/components/contributors/fragments/metric-dropdown.vue
- frontend/app/components/modules/collection/components/list/filters/collection-filter-industry.vue
- frontend/app/components/modules/project/components/contributors/config/metrics.ts
🧰 Additional context used
🧬 Code Definitions (16)
frontend/app/config/platforms/configs/git.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/jira.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/groupsio.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/twitter.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/linkedin.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/reddit.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/discord.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/slack.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/github.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/index.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/devto.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/gitlab.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/discourse.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/stackoverflow.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/gerrit.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
frontend/app/config/platforms/configs/hackernews.platform.ts (1)
frontend/types/shared/platforms.types.ts (1)
PlatformConfig(6-11)
🔇 Additional comments (53)
frontend/app/components/uikit/dropdown/dropdown-separator.vue (1)
1-9: Clean and well-structured Vue component.This new dropdown separator component follows proper Vue component structure with a clear template and component name definition. The component has a single purpose of rendering a visual separator in dropdown menus.
frontend/app/components/uikit/dropdown/dropdown-group-title.vue (1)
1-11: Properly implemented dropdown group title component.This component follows Vue best practices with a clear template including a slot for content insertion, and proper component naming. The structure aligns well with the dropdown-separator component, maintaining consistency across the dropdown UI kit.
frontend/app/config/platforms/configs/devto.platform.ts (2)
1-15:Details
✅ Verification successful
Platform configuration looks correct.
The Dev.to platform configuration follows the
PlatformConfiginterface structure and includes appropriate values for key, label, image, and activityTypes.One minor observation: Consider verifying that Dev.to only supports commenting as an activity type. If there are other activity types that should be supported, they would need to be added to the
activityTypesarray.
🏁 Script executed:
#!/bin/bash # Check if there are other activity types for Dev.to in the codebase echo "Checking for other Dev.to activity types in the codebase..." rg -t ts "ActivityTypes\..*.*\.valueOf\(\)" --context 5 | grep -B 5 -A 5 "DEVTO"Length of output: 1005
Dev.to Platform Configuration is Correct and Verified
The Dev.to configuration in
frontend/app/config/platforms/configs/devto.platform.tsfollows thePlatformConfiginterface correctly by using the appropriate key, label, image, and activityTypes. The shell script output confirms that only the commenting activity type is present, so no additional updates are needed at this time. If Dev.to ever supports more activity types, please update the configuration accordingly.
6-6:Details
✅ Verification successful
Verify enum value handling is consistent.
The use of
.valueOf()on the enum value is somewhat unusual. This is typically used to get the primitive value of an object, but for TypeScript enums, direct access usually suffices.
🏁 Script executed:
#!/bin/bash # Check how enum values are accessed across the codebase echo "Checking enum value access patterns..." rg -t ts "ActivityPlatforms\.[A-Z_]+\.valueOf\(\)" --context 2 echo "Checking for direct enum access in the codebase..." rg -t ts "ActivityPlatforms\.[A-Z_]+" --context 2 | grep -v "valueOf"Length of output: 16371
ACTION: Enum Handling Consistency Confirmed
I verified the enum usage across the codebase. In all platform configuration files—including
frontend/app/config/platforms/configs/devto.platform.ts—the key is explicitly set using.valueOf(), and this pattern is applied uniformly. Although using.valueOf()on a TypeScript enum is atypical when a direct enum reference would usually suffice, the approach appears deliberate for configuration keys (likely to ensure a primitive value is stored). In contrast, API calls and other parts of the codebase use the enum directly for comparisons, which is appropriate for that context.• All platform config files consistently use
.valueOf()on enum values.
• Direct enum access in the API and other logic remains unchanged and intentional.If the goal is to simplify the code by removing unnecessary primitive conversions—and the platform configuration type permits direct enum usage—you might consider refactoring. Otherwise, no changes are required.
frontend/app/config/platforms/configs/slack.platform.ts (2)
1-15: Platform configuration is properly structured.The Slack platform configuration adheres to the
PlatformConfiginterface with appropriate values for all required fields. The platform is configured with the MESSAGE activity type, which makes sense for a messaging platform like Slack.
6-6:Details
✅ Verification successful
Consider consistency in enum access.
Similar to the Dev.to configuration, the
.valueOf()method is used to access the enum value. Ensure this approach is used consistently across all platform configurations for maintainability.
🏁 Script executed:
#!/bin/bash # Check for consistency in platform configurations echo "Checking enum usage in other platform configurations..." fd -t f -e ts "\.platform\.ts$" -p "frontend/app/config/platforms/configs" --exec grep -l "valueOf" {} \; | wc -l echo "Total platform configuration files:" fd -t f -e ts "\.platform\.ts$" -p "frontend/app/config/platforms/configs" | wc -lLength of output: 405
Enum Access Consistency Verified
After reviewing the configurations, I confirmed that all 16 platform configuration files consistently use the
.valueOf()method for enum access. No changes are required in this regard.frontend/app/config/platforms/configs/groupsio.platform.ts (1)
1-15: Looks good - well-structured platform configuration for Groups.ioThe implementation follows the
PlatformConfiginterface correctly with all the required properties (key, label, image, and activityTypes). The activity types array is properly defined with the message activity type.frontend/app/config/platforms/index.ts (2)
1-17: Clean import structure for platform configurationsThe imports are organized in a readable manner, importing each platform configuration from its respective file.
19-36: Well-structured export of platform configurationsThe
platformsobject is properly typed as aRecord<string, PlatformConfig>and includes all the imported platform configurations. This approach makes it easy to access any platform by its key.frontend/types/shared/platforms.types.ts (2)
1-4: Clear and concise ActivityType interface definitionThe interface is well-defined with the essential properties needed for activity types.
6-11: Well-structured PlatformConfig interfaceThe
PlatformConfiginterface is properly defined with all the necessary properties for platform configuration. This provides good type safety for the platform configurations throughout the application.frontend/app/config/platforms/configs/discourse.platform.ts (1)
1-19: Well-implemented Discourse platform configurationThe implementation correctly follows the
PlatformConfiginterface with appropriate values for the Discourse platform. The activity types are properly defined for topic creation and messaging.frontend/app/config/platforms/configs/gitlab.platform.ts (1)
1-47: Well-structured platform configuration for GitLabThe GitLab platform configuration is well-implemented with appropriate activity types. The use of TypeScript enums (ActivityPlatforms and ActivityTypes) provides type safety, and the structure follows the PlatformConfig interface correctly.
frontend/app/config/platforms/configs/linkedin.platform.ts (1)
1-15: LinkedIn platform configuration implemented correctlyThe implementation follows the established pattern and adheres to the PlatformConfig interface.
LinkedIn typically supports more activity types than just commenting. Is this limitation intentional, or should additional activities like posting content or reactions be included in this configuration?
frontend/app/config/platforms/configs/discord.platform.ts (1)
1-23: Platform configuration for Discord looks well structuredThe configuration correctly defines Discord's key, label, and image path, along with three relevant activity types that match Discord's functionality: sending messages, starting threads, and sending messages within threads.
frontend/app/config/platforms/configs/confluence.platform.ts (2)
1-43: Platform configuration for Confluence looks well structuredThe configuration properly defines Confluence's key, label, and image path, along with appropriate activity types for Confluence's features.
18-41: Check for potential activity type redundancyThere appear to be potentially redundant activity types:
- Lines 19-21:
COMMENT_CREATED("Created a comment")- Lines 39-41:
COMMENT("Commented on a page")Similarly:
- Lines 23-25:
ATTACHMENT_CREATED("Created an attachment")- Lines 35-37:
ATTACHMENT("Attached a file")Verify if these represent distinct actions or if they could be consolidated.
frontend/app/config/platforms/configs/twitter.platform.ts (1)
1-19: Platform configuration for Twitter looks well structuredThe configuration correctly defines Twitter's key, label (using "X/Twitter" to reflect rebranding), image path, and two relevant activity types.
frontend/app/config/platforms/configs/git.platform.ts (1)
1-55: Platform configuration for Git is comprehensiveThe configuration properly defines Git's key, label, and image path, along with a thorough list of activity types covering various commit-related actions.
frontend/app/config/platforms/configs/jira.platform.ts (1)
1-39: Well-structured Jira platform configurationThe implementation follows the
PlatformConfiginterface correctly and provides a comprehensive set of activity types for Jira integration. The activity types are well-labeled and cover common Jira interactions.frontend/app/config/platforms/configs/stackoverflow.platform.ts (1)
1-19: Clean Stack Overflow platform configurationThe Stack Overflow configuration is correctly implemented and follows the established pattern for platform configurations. The activity types are appropriately defined for the platform's primary functions.
frontend/app/components/uikit/dropdown/dropdown-selector.vue (1)
1-36: Well-designed dropdown selector componentThe
LfxDropdownSelectorcomponent follows Vue 3 best practices with composition API and proper TypeScript typing. The component is flexible with customizable size and type props, and provides slot options for content customization.frontend/app/config/platforms/configs/github.platform.ts (1)
1-60: Well-structured platform configuration!This new GitHub platform configuration follows the
PlatformConfiginterface correctly and provides a comprehensive list of GitHub-specific activity types with clear, user-friendly labels.The use of
valueOf()on enum values ensures proper string conversion, and the organization of activity types is logical, grouping related activities together (commits, pull requests, issues, discussions).frontend/app/components/modules/project/components/contributors/geographical-distribution.vue (2)
15-29: Improved responsive layout with activities dropdown!The new layout using
flex-wrapandflex-nowrapwithjustify-betweenimproves responsiveness across different screen sizes. The replacement oflfx-metric-dropdownwithlfx-activities-dropdownaligns with the PR objective of refactoring metrics to activities.The addition of placement, width, and other configuration options to the dropdown component provides better control over its appearance and behavior.
95-95: Import updated to match component changeThe import statement has been correctly updated to match the new component name in the template.
frontend/app/components/modules/project/components/contributors/contributors-leaderboard.vue (1)
16-16: Component replacement looks good.The metric dropdown has been successfully replaced with the new activities dropdown component while preserving the same v-model binding to
metric. This is consistent with the PR objective of refactoring metrics to activities.Also applies to: 39-39
frontend/app/components/modules/project/components/contributors/organization-dependency.vue (1)
17-17: Component replacement is implemented correctly.The activities dropdown is properly integrated, maintaining the same v-model binding to
metric. The import statement is also correctly updated with proper formatting across multiple lines.Also applies to: 66-67
frontend/app/components/modules/project/components/contributors/organizations-leaderboard.vue (1)
16-16: Component replacement is consistent with other files.The metric dropdown has been properly replaced with the activities dropdown while maintaining the same data binding pattern used in other components. This ensures consistent behavior across the application.
Also applies to: 40-40
frontend/app/components/uikit/popover/popover.vue (5)
5-6: Good addition of dynamic class and attribute binding.The
is-openclass binding provides visual state feedback, and thev-bind="$attrs"ensures any additional attributes are properly passed to the trigger element, improving component flexibility.
17-18: Modal functionality is correctly implemented.The conditional class and click handler for the modal mode provide a clean way to implement different popover behaviors. This is a good enhancement that adds flexibility to the component.
41-42: New props enhance component versatility.The addition of
matchWidthandisModalprops with sensible defaults extends the component's functionality without breaking existing usage. These additions support the dropdown refactoring goals.Also applies to: 49-50
77-89: Width matching functionality is well implemented.The conditional modifier for matching the popover width to the reference element is a valuable addition. This is particularly important for dropdown components where consistent width improves usability.
147-150: Good exposure of component methods.Exposing the
closePopoverandopenPopovermethods throughdefineExposeallows parent components to programmatically control the popover, which enhances component interoperability.frontend/app/components/uikit/dropdown/dropdown-select.vue (1)
1-17: Well-structured dropdown component implementationThe dropdown component is well-designed, using slots effectively for customization and proper v-binding for attributes. The approach of having a separate trigger slot for customization follows good component design patterns.
frontend/app/components/modules/collection/views/collection-list.vue (2)
28-51: Good transition to declarative dropdown structureThe replacement of the old dropdown with the new dropdown-select component improves code readability by making the options more declarative. The trigger customization with the dropdown-selector provides a consistent UI pattern.
125-127: Appropriate component importsThe imports have been correctly updated to include the new dropdown components.
frontend/app/components/modules/project/components/contributors/contributor-dependency.vue (3)
17-17: Consistent dropdown replacementThe metric dropdown has been successfully replaced with the activities dropdown while maintaining the same v-model binding. This ensures backward compatibility with the existing data flow.
68-69: Updated import for activities dropdownThe import statement has been properly updated to reflect the component rename from metrics to activities.
79-81:Details
❓ Verification inconclusive
Verify all usages with new dropdown terminology
The variable name
metricis still used even though the component has been renamed to activities dropdown. This is likely intentional to maintain API compatibility, but ensure all usages are consistent.
🏁 Script executed:
#!/bin/bash # Description: Verify if metric and activity terminology are used consistently # Look for usages of metric and activity in the codebase echo "Files using both 'metric' and 'activity' terminology:" rg -l "metric.*activity|activity.*metric" --type vueLength of output: 211
Action Required: Verify Consistency in Terminology for the Activities Dropdown
It appears that our initial search script did not run as expected due to an unrecognized file type (“vue”). This raises uncertainty about whether the “metric” variable and its derivatives are consistently used throughout the component now known as the activities dropdown. If keeping “metric” is an intentional decision for API compatibility, please ensure that this exception is clearly documented. Otherwise, update all relevant usages to reflect the new terminology.
To aid your verification, please run the following revised shell script to search all *.vue files for occurrences where “metric” and “activity” are used together:
#!/bin/bash # Searching for files with both 'metric' and 'activity' terminology in .vue files: echo "Searching for files with both 'metric' and 'activity' usage in .vue files:" rg -l "metric.*activity|activity.*metric" -g "*.vue"Once you have verified the output, adjust the code or documentation accordingly.
frontend/app/components/modules/project/components/contributors/fragments/activities-dropdown.vue (1)
1-59: Well-structured activities dropdown componentThe dropdown implementation is clean and effectively organizes activities by platform. The use of icons for visual enhancement and the grouping structure provides good UX. The fullWidth prop with conditional classes is a nice touch for responsive design.
frontend/app/components/modules/collection/components/details/filters.vue (2)
41-67: Refactor to ensure consistent sorting logic.
This dropdown-based approach effectively replaces a data-driven list with a declarative structure. Ensure that all possible sorting modes are accounted for and that unused modes have been removed, preventing confusion.Would you like to confirm references to these sort modes throughout the codebase? I can provide a script to search for any deprecated sort keys.
76-79: Confirm that all imports are registered in script setup.
These imported components (LfxDropdownSelector,LfxDropdownSelect,LfxIcon,LfxDropdownItem) appear to be utilized directly in the<template>without explicit component registration, which works under<script setup>. This is valid, but ensure there are no conflicts with similarly named components in the global scope.frontend/app/components/modules/project/components/shared/header/date-range-picker.vue (4)
3-21: Prefer consistent custom value handling.
The trigger template logic for displaying a custom date range is clear and user-friendly. Consider ensuring that any other “custom” scenarios (like partial date selection) are handled gracefully or validated.
65-85: Maintain user context consistency.
Including a separator between date ranges is a good way to visually organize the options. Double-check that the logic for each range is consistent in start and end dates, especially if your platform’s usage patterns differ across “past” vs. “previous” vs. “general” segments.
106-106: Ensure date library compatibility.
Your usage of Luxon requires correct imports and consistent date formatting. Make sure this file and any other date manipulations across the application rely on the same library and configured locale/timezone to avoid confusion.
129-130: Revisit default range selection.
Defaulting topast365daysis typically fine for an analytics-focused interface. However, confirm that this default aligns with user expectations or typical usage patterns in your specific business logic.frontend/app/components/uikit/dropdown/dropdown.vue (4)
2-7: Conditionally modal for mobile view.
Using:is-modal="pageWidth < 640"is a straightforward approach. Just verify that the responsive logic remains consistent across the rest of your application for a uniform mobile user experience.
9-10: Styling the trigger slot.
Providing a slot named"trigger"is good for flexibility. Confirm that all usage sites inject the correct elements here, especially icons or text that may require spacing or specific class names.
11-20: Close popover on item click.
Callingpopover.closePopover()ensures the dropdown closes when an item is clicked. Verify that any advanced behaviors (e.g., multi-select or partial selection) handle closing logic differently if needed.
24-49: Emit a visibility update.
The computed setter forisVisiblecorrectly emits'update:visibility'. If multiple dropdown/popover instances can exist simultaneously, ensure no collisions in controlling visibility states or event name collisions.frontend/app/components/uikit/dropdown/dropdown.scss (2)
28-30: Separator Styling Looks Good
The rules for&__separatorprovide a clear visual separation between groups of items. The use of utility classes keeps the implementation straightforward.
32-34: Group Title Styling Is Consistent
The styling for&__group-titleis concise and matches the overall design language.frontend/types/shared/activity-platforms.ts (1)
1-20: Standardized Enum Values for Consistency
The changes in theActivityPlatformsenum—updating all platform string values to lowercase—improve consistency across the application. Ensure that all references to these enum values in configurations and components (e.g., dropdowns) have been updated accordingly.
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
In this PR
ℹ️ This PR is part of a stack
Please review them in order, as some of the first ones set the foundation for the later ones.
Summary by CodeRabbit
New Features
Refactor
Chores