Skip to content

Conversation

@dgarcia360
Copy link
Collaborator

@dgarcia360 dgarcia360 commented Nov 21, 2025

What changes are proposed in this pull request?

Closes #6590 (comment)

How is this patch tested? If it is not, please explain why.

Tested locally.

Release Notes

Is this a user-facing change that should be mentioned in the release notes?

  • No. You can skip the rest of this section.
  • Yes. Give a description of this change to be included in the release
    notes for FiftyOne users.

(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)

What areas of FiftyOne does this PR affect?

  • App: FiftyOne application changes
  • Build: Build and test infrastructure changes
  • Core: Core fiftyone Python library changes
  • Documentation: FiftyOne documentation changes
  • Other

Summary by CodeRabbit

  • Style
    • Updated documentation layout styling to conditionally adjust article container width when secondary sidebars are present.

✏️ Tip: You can customize this high-level summary in your review settings.

@dgarcia360 dgarcia360 requested a review from a team as a code owner November 21, 2025 09:17
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 21, 2025

Walkthrough

Two CSS styling updates are applied: duplicate font-face declarations are added to custom CSS, and the article container max-width rule is modified to conditionally apply only when a secondary sidebar element is present in the DOM.

Changes

Cohort / File(s) Summary
CSS Font Declarations
docs/source/_static/css/custom.css
Adds duplicate @font-face blocks for Geist and Geist Mono fonts, appending them to existing font declarations
Layout Conditional Rules
docs/source/assets/styles/layout/_main.sass
Replaces unconditional max-width: 50em rule on .bd-article-container with conditional rule that applies max-width only when .bd-sidebar-secondary element is present via :has() selector

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Attention areas:
    • Verify intent of duplicate font-face declarations in custom.css — confirm this is not accidental redundancy
    • Validate that :has() selector is supported across target browsers for layout rule in _main.sass
    • Test responsive behavior with and without secondary sidebar present

Poem

🐰 A rabbit hops through styles so clean,
Duplicating fonts in the CSS scene,
While sidebars dance with conditional flair,
Layout rules bloom with :has() care! 🎨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: increasing content width on documentation pages without a right sidebar.
Description check ✅ Passed The description follows the required template with all main sections completed: proposed changes, testing approach, and release notes determination.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch docs-increase-width

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58b5b03 and f6c4103.

📒 Files selected for processing (2)
  • docs/source/_static/css/custom.css (1 hunks)
  • docs/source/assets/styles/layout/_main.sass (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: build
🔇 Additional comments (3)
docs/source/_static/css/custom.css (2)

75-75: Verify browser compatibility for :has() selector.

The change uses the :has() pseudo-class selector, which has variable browser support:

  • Chrome/Edge 105+ (improved in later versions)
  • Firefox 121+ (behind experimental flag)
  • Safari 15.4+
  • Internet Explorer: not supported

Ensure this meets your minimum browser support requirements. If support for older browsers is needed, you may need a fallback approach.


1-1: Clarify the role of custom.css in the build process.

This appears to be a minified CSS file that duplicates rules from _main.sass. Please confirm:

  • Is this file auto-generated from SASS during the build?
  • Or is it manually maintained alongside the SASS source?
  • Why is it being added to the repository rather than generated at build time?

Maintaining the same rules in both files risks inconsistencies and makes updates harder to track.

docs/source/assets/styles/layout/_main.sass (1)

75-76: Clean conditional styling approach.

The use of :has() to conditionally apply the max-width constraint is a clean way to achieve the goal. However, the same browser compatibility concern noted in custom.css applies here—verify that :has() support aligns with your project's minimum browser support policy.

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@AdonaiVera AdonaiVera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, LGTM. Thanks for jumping on the fix so fast, David!

@AdonaiVera AdonaiVera merged commit f401778 into main Nov 21, 2025
15 checks passed
@AdonaiVera AdonaiVera deleted the docs-increase-width branch November 21, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants