-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Feature/ux improvements - This PR updates a line from PR #3091 causing wrong brand color. Apologies for the oversight! #3092
Feature/ux improvements - This PR updates a line from PR #3091 causing wrong brand color. Apologies for the oversight! #3092
Conversation
…n pages in customs.css
WalkthroughThe pull request focuses on updating the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Learnings (1)docs/src/css/custom.css (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (4)
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
Documentation and Community
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #3092 +/- ##
=================================================
Coverage 39.85% 39.85%
=================================================
Files 453 453
Lines 33203 33203
Branches 385 385
=================================================
Hits 13234 13234
Misses 19969 19969 ☔ View full report in Codecov by Sentry. |
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: 5
🔭 Outside diff range comments (2)
docs/src/css/index.css (1)
Fix variable name mismatch and remove unrelated changes
The implementation has two issues:
- The defined variable
--brand
is never used, while the code references a non-existent--brand-color
- The PR includes many unrelated color and UI variables that should be part of a separate change
🔗 Analysis chain
Line range hint
8-25
: Review brand color implementation.The brand color is now properly defined as a root variable:
--brand: #febc59;However, this PR includes many additional color and UI variables beyond the scope of the brand color fix.
Please confirm if these additional variable definitions are necessary for the brand color fix.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Search for usage of the brand color variable rg --type css "var\(--brand\)" -B 2 -A 2 # Search for any potential color calculations or modifications involving brand color rg --type css "color:.*--brand|background.*--brand|border.*--brand" -B 2 -A 2 # Look for any CSS files that might contain related styling fd -e css -x cat {} \; | rg "febc59" -B 2 -A 2Length of output: 436
docs/src/css/custom.css (1)
Line range hint
599-611
: Use CSS custom properties for button gradients.The gradient values are duplicated across multiple button styles. Consider using CSS custom properties for better maintainability.
+ :root { + --button-gradient: linear-gradient(90deg, #ff3600 0%, #ff8100 100%); + --button-gradient-hover: linear-gradient(90deg, #ff3600 30%, #ff8100 78%); + } .markdown > button { - background: linear-gradient(90deg, #ff3600 0%, #ff8100 100%); + background: var(--button-gradient); /* ... */ } .markdown > button:hover { - background: linear-gradient(90deg, #ff3600 30%, #ff8100 78%); + background: var(--button-gradient-hover); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (26)
docs/docusaurus.config.ts
(2 hunks)docs/src/components/HomepageFeatures/index.tsx
(0 hunks)docs/src/components/HomepageFeatures/styles.module.css
(0 hunks)docs/src/components/animations/_dissectionAnimation.js
(0 hunks)docs/src/components/animations/_headerAnimation.js
(0 hunks)docs/src/components/layout/EightPanel.tsx
(0 hunks)docs/src/components/layout/FifthPanel.tsx
(0 hunks)docs/src/components/layout/FourthPanel.tsx
(0 hunks)docs/src/components/layout/HeaderHero.tsx
(1 hunks)docs/src/components/layout/SecondPanel.tsx
(0 hunks)docs/src/components/layout/SeventhPanel.tsx
(0 hunks)docs/src/components/layout/SixthPanel.tsx
(0 hunks)docs/src/components/layout/ThirdPanel.tsx
(0 hunks)docs/src/css/custom.css
(10 hunks)docs/src/css/index.css
(7 hunks)docs/src/hooks/useHomePageAnimations.ts
(0 hunks)docs/src/pages/index.tsx
(1 hunks)docs/src/pages/markdown-page.md
(0 hunks)docs/src/utils/AppFeaturesCard.tsx
(0 hunks)docs/src/utils/HomeCallToAction.tsx
(1 hunks)docs/src/utils/ManagementFeaturesCard.tsx
(0 hunks)docs/src/utils/OrganizationFeatureCard.tsx
(0 hunks)docs/src/utils/ParticipationFeatureCard.tsx
(0 hunks)docs/src/utils/TextColumn.tsx
(0 hunks)docs/src/utils/TwoColumns.tsx
(0 hunks)docs/src/utils/textcontent.js
(0 hunks)
💤 Files with no reviewable changes (20)
- docs/src/pages/markdown-page.md
- docs/src/components/layout/SecondPanel.tsx
- docs/src/components/layout/FifthPanel.tsx
- docs/src/utils/TextColumn.tsx
- docs/src/utils/AppFeaturesCard.tsx
- docs/src/utils/OrganizationFeatureCard.tsx
- docs/src/components/HomepageFeatures/styles.module.css
- docs/src/components/layout/EightPanel.tsx
- docs/src/components/layout/FourthPanel.tsx
- docs/src/utils/TwoColumns.tsx
- docs/src/utils/ParticipationFeatureCard.tsx
- docs/src/components/layout/SeventhPanel.tsx
- docs/src/components/layout/ThirdPanel.tsx
- docs/src/components/HomepageFeatures/index.tsx
- docs/src/components/layout/SixthPanel.tsx
- docs/src/utils/ManagementFeaturesCard.tsx
- docs/src/hooks/useHomePageAnimations.ts
- docs/src/components/animations/_headerAnimation.js
- docs/src/utils/textcontent.js
- docs/src/components/animations/_dissectionAnimation.js
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Run tests for talawa api
- GitHub Check: Analyse Code With CodeQL (typescript)
🔇 Additional comments (9)
docs/src/components/layout/HeaderHero.tsx (2)
9-13
: Changes appear more extensive than described in PR objectives.The PR description mentions fixing a brand color issue, but these changes involve significant restructuring of the HeaderHero component, including:
- Removal of two-column layout
- Removal of image elements
- Updates to text content
Please clarify if these additional changes are intentional and related to the brand color fix.
14-16
: LGTM: Button implementation looks good.The HomeCallToAction button implementation is clean and properly structured.
docs/src/pages/index.tsx (1)
Line range hint
7-21
: Verify intentional removal of homepage components.The changes include removal of multiple panel components and animations. This seems to be a significant departure from the stated PR objective of fixing a brand color issue.
Please confirm if removing these components is intentional:
- SecondPanel through EighthPanel components
- Homepage animations
- Associated imports
docs/src/utils/HomeCallToAction.tsx (1)
19-19
: LGTM: Improved GitHub link specificity.Good improvement to make the GitHub link point directly to the talawa-api repository instead of the organization.
docs/src/css/index.css (1)
159-192
: Verify necessity of media query changes.The PR includes extensive responsive design changes that appear unrelated to the brand color fix:
- New breakpoints at 760px, 480px, and 960px
- Layout adjustments for different screen sizes
Please clarify if these responsive design changes are necessary for the brand color fix.
docs/src/css/custom.css (1)
73-73
: LGTM: Dark theme configuration looks good.The dark theme variables are well-structured and maintain proper contrast ratios for accessibility.
docs/docusaurus.config.ts (3)
87-88
: LGTM! Navigation configuration follows Docusaurus best practices.The change from
href
toto
withactiveBasePath
is the correct approach for internal navigation in Docusaurus. This ensures proper active link highlighting and client-side routing.
165-165
: LGTM! Copyright notice updated correctly.The copyright notice has been appropriately updated to reflect The Palisadoes Foundation, LLC.
Line range hint
1-174
: Verify the brand color changes mentioned in PR objectives.The PR objectives mention fixing a brand color issue and updating
.navbar__brand
color, but no color-related changes are visible in this file. The color changes might be in CSS files that weren't included in this review.Let's verify the color changes:
Please make coderabbit approve your work |
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Please make code rabbit approve your PR |
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
docs/src/css/custom.css
(10 hunks)
🧰 Additional context used
📓 Learnings (1)
docs/src/css/custom.css (2)
Learnt from: Abhi-MS
PR: PalisadoesFoundation/talawa-api#3092
File: docs/src/css/custom.css:0-0
Timestamp: 2025-01-28T06:14:47.383Z
Learning: When hiding elements that should remain accessible to screen readers in Talawa's documentation, use the clip-pattern (position: absolute, width/height: 0, clip/clip-path) instead of display: none.
Learnt from: Abhi-MS
PR: PalisadoesFoundation/talawa-api#3092
File: docs/src/css/custom.css:656-683
Timestamp: 2025-01-28T06:09:27.385Z
Learning: Media query consolidation in docs/src/css/custom.css should preserve both min-width and max-width constraints to properly handle narrow and wide screen edge cases. The file uses specific ranges (e.g. 1001px-1049px) to target precise screen sizes.
🔇 Additional comments (6)
docs/src/css/custom.css (6)
71-76
: LGTM! Icon dimensions are now properly managed using CSS variables.The implementation follows the suggested improvements from previous reviews, making icon dimensions more maintainable.
289-315
: LGTM! Icon dimensions now use the CSS variables.The icon dimensions are now properly referenced using the CSS variables, improving maintainability.
651-652
: LGTM! HeaderHero height is now responsive using clamp.The implementation follows the suggested improvements from previous reviews, using CSS clamp for fluid responsive heights.
656-666
: LGTM! Accessibility-friendly approach to hiding SVG icons.The implementation follows the clip-pattern for hiding elements while keeping them accessible to screen readers, as discussed in previous reviews.
Line range hint
669-773
: LGTM! Media queries maintain precise screen size control.The implementation preserves both min-width and max-width constraints as required, ensuring proper handling of narrow and wide screen edge cases.
Line range hint
46-46
: LGTM! Brand color is now correctly implemented.The brand color is properly defined for both light and dark themes, addressing the issue mentioned in the PR objectives.
Also applies to: 91-91
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
CONTRIBUTING.mdRead our CONTRIBUTING.md file. Most importantly:
Other
|
Its good now. |
7d4f843
into
PalisadoesFoundation:develop-postgres
What kind of change does this PR introduce?
.navbar__brand colour was changed. Fixing this issue
Issue Number:
Fixes #2951
Snapshots/Videos:
Before,
After,
If relevant, did you update the documentation?
Not relevant
Summary
Brand colour was changed as suggested by coderabitai which led to unwanted changes.
Does this PR introduce a breaking change?
No.
Checklist
CodeRabbit AI Review
Test Coverage
Other information
Have you read the contributing guide?
Yes
Summary by CodeRabbit
.HeaderHero
class to use responsive height withclamp()