fix social mentions styling and added new type param#129
Conversation
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
WalkthroughThis PR implements updates to the social mentions feature across both the frontend and backend. The social mentions component now displays updated text, a "Learn more" link, and a skeleton loading state during data fetching. It features dynamic chart rendering that switches between line and bar charts based on a renamed active tab and computed granularity values. The popularity view integrates the social mentions section with updated side navigation and imports. On the backend, the API endpoint now uses a dynamic Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as Social Mentions Component
participant L as Load State Manager
participant API as Social Mentions API
participant Chart as Chart Renderer
U->>C: Navigate to Social Mentions section
C->>L: Display skeleton loading state
C->>API: Request social mentions data (granularity, type)
API-->>C: Return data (cumulative or new mentions)
C->>Chart: Supply data and computed chart configuration
Chart-->>C: Render appropriate chart (line/bar)
C->>U: Update UI with social mentions summary and chart
Possibly related PRs
🪧 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 pull request fixes the social mentions widget by updating the styling, chart colors, and introduces a new parameter for type filtering (cumulative vs new mentions). Key changes include:
- Adding a yellow color alias in the color configuration.
- Updating the API and UI to support a new type parameter, changing the condition from 'all-time' to 'cumulative'.
- Adjusting the social mentions widget component and view to match the new design and functionality.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| frontend/app/config/styles/colors.ts | Added a new yellow color alias with a TODO comment. |
| frontend/server/api/project/[slug]/popularity/social-mentions.get.ts | Updated query parameters (granularity and type) and condition logic to use 'cumulative'. |
| frontend/server/mocks/social-mentions.mock.ts | Renamed date fields for consistency (from dateFrom/dateTo to startDate/endDate). |
| frontend/app/components/modules/project/components/popularity/social-mentions.vue | Updated UI styling, error handling, and chart configuration for social mentions. |
| frontend/app/components/modules/project/views/popularity.vue | Integrated the social mentions widget into the project view. |
Comments suppressed due to low confidence (1)
frontend/app/components/modules/project/components/popularity/social-mentions.vue:159
- The API documentation specifies the type parameter for new mentions as 'new', but the UI tab is using 'new-mentions'. This mismatch may lead to unexpected behavior when fetching data for new mentions.
{ label: 'New mentions by platform', value: 'new-mentions' }
frontend/app/components/modules/project/components/popularity/social-mentions.vue
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
frontend/server/api/project/[slug]/popularity/social-mentions.get.ts (1)
36-40: 🛠️ Refactor suggestionAdd validation for the type parameter
The code assumes
query.typewill always be either 'cumulative' or some other value but lacks validation to ensure it's one of the expected values.Add validation for the
typeparameter to prevent unexpected behavior:- if (query.type === 'cumulative') { + const type = ['cumulative', 'new'].includes(query.type as string) + ? query.type as 'cumulative' | 'new' + : 'cumulative'; // Default value + + if (type === 'cumulative') {frontend/server/mocks/social-mentions.mock.ts (1)
148-149:⚠️ Potential issueIncomplete property renaming in the newMentions summary object
While most date properties have been renamed from
dateFrom/dateTotostartDate/endDate, theperiodFromandperiodToproperties in thenewMentions.summaryobject were not updated consistently.Apply this diff to maintain consistency with the renamed properties:
summary: { current: 4000, previous: 3880, percentageChange: 3.1, changeValue: 120, - periodFrom: '2024-11-11T00:00:00Z', - periodTo: '2025-02-12T00:00:00Z' + startDate: '2024-11-11T00:00:00Z', + endDate: '2025-02-12T00:00:00Z' },
🧹 Nitpick comments (4)
frontend/app/config/styles/colors.ts (1)
86-87: Address the TODO comment and ensure color structure consistencyThe addition of the
yellowcolor with a TODO comment suggests this is a temporary or uncertain solution. Additionally, this single color definition is inconsistent with the structured palette approach used for other colors (which use shade levels 50-900).Consider either:
- Resolving the TODO by confirming the appropriate color alias with Nuno
- Structuring this color consistently with other colors by using a palette object:
- // TODO: Verify with Nuno what color alias we should use - yellow: '#FFD6A7' + amber: { + 500: '#FFD6A7', + // Add other shades as needed + }frontend/server/api/project/[slug]/popularity/social-mentions.get.ts (1)
26-28: Improve API documentation for the new parametersThe JSDoc for query parameters has been updated but lacks specific details about valid values for
granularityand potential usage examples.Enhance the documentation by providing allowed values and examples:
* Query params: * - granularity: string * - type: 'cumulative' | 'new' + * Examples: + * - granularity: 'day', 'week', 'month', etc.frontend/server/mocks/social-mentions.mock.ts (1)
67-74: Remove duplicate data entryLines 67-74 and 75-82 appear to contain duplicate data entries with identical date ranges and values.
Consider removing one of these duplicate entries to maintain cleaner mock data:
{ startDate: '2024-12-23T00:00:00Z', endDate: '2025-01-06T00:00:00Z', twitter: 1500, reddit: 1700, hackerNews: 1800, stackOverflow: 1900 }, - { - startDate: '2024-12-23T00:00:00Z', - endDate: '2025-01-06T00:00:00Z', - twitter: 1500, - reddit: 1700, - hackerNews: 1800, - stackOverflow: 1900 - },frontend/app/components/modules/project/components/popularity/social-mentions.vue (1)
194-194: Consider updating Stack Overflow color for better distinctionThe Stack Overflow series is using
lfxColors.warning[200]which might be too similar to the Hacker News color (lfxColors.warning[500]). Consider usinglfxColors.yellowinstead to match the legend color shown on line 65.- color: lfxColors.warning[200] + color: lfxColors.yellow
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/app/components/modules/project/components/popularity/social-mentions.vue(3 hunks)frontend/app/components/modules/project/views/popularity.vue(3 hunks)frontend/app/config/styles/colors.ts(1 hunks)frontend/server/api/project/[slug]/popularity/social-mentions.get.ts(2 hunks)frontend/server/mocks/social-mentions.mock.ts(2 hunks)
🔇 Additional comments (9)
frontend/app/components/modules/project/views/popularity.vue (2)
30-35: The social mentions widget implementation looks goodThe addition of the social mentions component follows the established pattern of the other sections and is properly integrated with the scroll functionality.
58-58: Clean implementation of the imports and navigation itemsThe import of the
LfxProjectSocialMentionscomponent and the addition of the navigation item are properly implemented.Also applies to: 71-71
frontend/app/components/modules/project/components/popularity/social-mentions.vue (7)
7-12: Good addition of dynamic project name and "Learn more" linkThe updated text now provides context-specific information by using the project name, and the "Learn more" link gives users access to additional information. This makes the component more helpful and user-friendly.
16-34: Great improvement to loading state implementationThe skeleton loading state implementation is a significant UX improvement over the previous approach. Using the
lfx-skeleton-statecomponent provides better visual feedback during data loading, and checking for the existence ofsummaryis more robust than relying on a fixed status check.
42-72: Excellent chart rendering implementationThe new chart rendering approach with
lfx-project-load-statehandles loading, error, and empty states much more effectively. The dynamic chart selection based on the active tab is well-implemented, and the custom legend with color indicators for each platform improves user understanding of the data.
120-128: Well-structured granularity computationThe computed properties for granularity handling are well-designed, providing excellent flexibility for different chart types and time ranges. The separation of
barGranularityandlineGranularitywith a unifiedgranularityproperty is a clean implementation that adapts to both custom date ranges and predefined time periods.
134-135: Good update to API request parametersChanging from a static
intervalto a dynamicgranularityparameter and adding thetypeparameter based onactiveTabaligns well with the backend changes and provides more flexible data fetching.
148-148: Good standardization of date property namesUpdating the property name from 'date' to 'startDate' in the
convertToChartDatafunction ensures consistency with the standardized date property names in the API response.
163-206: Great chart configuration improvementsThe dynamic chart type selection in
chartSeriesand the separate configurations for line and bar charts provide more appropriate visualizations based on the context. The implementation correctly uses the computed granularity values and ensures a consistent user experience.
| startDate: '2024-11-11T00:00:00Z', | ||
| endDate: '2024-01-31T00:00:00Z', |
There was a problem hiding this comment.
Fix inconsistent date range in mock data
There's an issue with the date range in the first entry of newMentions.data: the end date (2024-01-31) is before the start date (2024-11-11), which is logically incorrect.
Correct the date range to ensure the end date is after the start date:
{
startDate: '2024-11-11T00:00:00Z',
- endDate: '2024-01-31T00:00:00Z',
+ endDate: '2024-11-30T00:00:00Z',
twitter: 500,
reddit: 700,
hackerNews: 800,
stackOverflow: 900
},📝 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.
| startDate: '2024-11-11T00:00:00Z', | |
| endDate: '2024-01-31T00:00:00Z', | |
| { | |
| startDate: '2024-11-11T00:00:00Z', | |
| endDate: '2024-11-30T00:00:00Z', | |
| twitter: 500, | |
| reddit: 700, | |
| hackerNews: 800, | |
| stackOverflow: 900 | |
| }, |
| startDate: '2024-02-01T00:00:00Z', | ||
| endDate: '2024-02-29T00:00:00Z', |
There was a problem hiding this comment.
Fix year in date range
The months in this mock data appear to be sequential, but there's an inconsistency in the year. If we're starting with November 2024, then February should also be 2025, not 2024.
Correct the year in the date range:
{
- startDate: '2024-02-01T00:00:00Z',
- endDate: '2024-02-29T00:00:00Z',
+ startDate: '2024-12-01T00:00:00Z',
+ endDate: '2024-12-31T00:00:00Z',
twitter: 300,
reddit: 500,
hackerNews: 600,
stackOverflow: 700
},This correction assumes we want consecutive months. Adjust according to the actual requirements for your mock data timeline.
📝 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.
| startDate: '2024-02-01T00:00:00Z', | |
| endDate: '2024-02-29T00:00:00Z', | |
| { | |
| startDate: '2024-12-01T00:00:00Z', | |
| endDate: '2024-12-31T00:00:00Z', | |
| twitter: 300, | |
| reddit: 500, | |
| hackerNews: 600, | |
| stackOverflow: 700 | |
| }, |
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/app/components/modules/project/components/popularity/social-mentions.vue (1)
65-66: Color inconsistency between template and script.There's a potential mismatch between the color definitions. The template uses
bg-yellowfor Stack Overflow, while the script useslfxColors.warning[200].Ensure consistency by using the same color reference:
- <div class="w-3 h-3 bg-yellow rounded-xs" /> + <div class="w-3 h-3 bg-warning-200 rounded-xs" />Or alternatively update the script to use the consistent color:
- color: lfxColors.warning[200] + color: lfxColors.yellowAlso applies to: 194-195
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app/components/modules/project/components/popularity/social-mentions.vue(3 hunks)
🔇 Additional comments (16)
frontend/app/components/modules/project/components/popularity/social-mentions.vue (16)
7-12: Clear and informative description with helpful "Learn more" link.The updated description clearly explains what social mentions represent, using the dynamic project name through the
keywordcomputed property. The addition of the "Learn more" link is a good practice for providing users with additional context.
16-34: Improved loading experience with skeleton state.The skeleton loading state implementation provides better user feedback during data fetching. The conditional rendering based on summary availability is properly implemented, and the delta display logic correctly accounts for the time range selection.
42-46: Error message consistency with component purpose.The error message now correctly refers to "social mentions" rather than "forks" as mentioned in a previous review.
48-71: Well-structured chart with appropriate legend.The dynamic chart selection based on the active tab is well-implemented. The legend with distinct colors for different platforms enhances usability and data comprehension.
94-94: Added import for line chart configuration.The addition of the line area chart import supports the new chart type functionality.
100-104: Required imports for new functionality.These imports provide the necessary functionality for links, date options, data validation, and granularity definitions needed for the improved component.
107-115: Enhanced store integration with keyword computation.The expanded destructuring of the store provides access to all required reactive properties. The computed keyword property intelligently uses the project name.
117-117: Tab renaming from 'all-time' to 'cumulative'.The default tab has been renamed to be more descriptive of its function.
120-129: Well-implemented dynamic granularity calculations.The computed properties for granularity selection based on time range and chart type are well-structured. The code correctly handles custom date ranges by leveraging the store's
customRangeGranularity.
134-135: API parameters updated to use dynamic values.The request parameters now use computed granularity and type values, providing more flexible and context-aware data fetching.
148-148: Chart data conversion using 'startDate' property.The
convertToChartDatafunction now properly uses 'startDate' as the date property key.
156-157: Added empty state detection.The isEmpty computed property helps determine when to display empty state UI, improving user experience.
159-159: Updated tab label for clarity.The tab has been relabeled to "Mentions by platform" which better describes the content being displayed.
163-196: Dynamic chart type based on selected tab.The chart series now dynamically changes its visualization type based on the active tab, switching between line and bar charts as appropriate. This provides different views of the data that may be better suited for different analysis needs.
197-201: Added line chart configuration.The new computed property for line chart configuration supports the dual visualization approach, using appropriate granularity values based on the selected time range.
205-205: Bar chart configuration updated for dynamic granularity.The bar chart configuration now correctly uses the computed barGranularity value, ensuring consistent time intervals across visualizations.
In this PR
Ticket
INS-93
Summary by CodeRabbit
New Features
Style