Skip to content

fix(system-theme): adiciona suporte ao tema claro#371

Open
fernanduandrade wants to merge 1 commit into
he4rt:4.xfrom
fernanduandrade:fix/system-theme
Open

fix(system-theme): adiciona suporte ao tema claro#371
fernanduandrade wants to merge 1 commit into
he4rt:4.xfrom
fernanduandrade:fix/system-theme

Conversation

@fernanduandrade

Copy link
Copy Markdown

Summary

  • Adiciona suporte ao tema claro nas rotas que possuíam compatibilidade apenas com o tema escuro
  • Corrige cores de textos e elementos da interface que ficavam ilegíveis quando o sistema do usuário estava configurado para o tema claro
  • Garante que as páginas afetadas respeitem o sistema de temas da aplicação, mantendo consistência visual entre os modos claro e escuro

@fernanduandrade fernanduandrade requested a review from a team June 23, 2026 23:41
@GustavoSimao

Copy link
Copy Markdown
Member

lgtm

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Removes the hardcoded class="dark" from the root <html> element and adds a meta name="color-scheme" content="light dark" tag. Extends the SocialLink DTO with an optional accentDark property and updates X/GitHub entries to supply a white secondary accent. Reworks .d-pill CSS to use a --accent-resolved variable derived from --accent-light/--accent-dark, updates navbar and logo components to use zinc-based dark-mode utilities and color-mix instead of hardcoded white values, and narrows the .hp-button-icon hover selector to exclude icon-only variants. Adds and updates feature tests to verify new markup and CSS variable naming.

Possibly related PRs

  • he4rt/heartdevs.com#204: Touches the same portal layout and navbar Blade templates for dark-mode/theme markup.
  • he4rt/heartdevs.com#340: Introduced the /redes social links page; this PR extends it with accentDark, revised CSS variables, and updated test assertions.

Suggested labels

mod:portal

Suggested reviewers

  • Clintonrocha98
  • gvieira18
  • Luisnadachi
  • 1pride
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(system-theme): adiciona suporte ao tema claro' clearly describes the main change: adding light theme support to the system.
Description check ✅ Passed The description is directly related to the changeset, detailing light theme support additions, text color fixes, and theme consistency improvements across multiple components.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
app-modules/portal/tests/Feature/SocialLinksPageTest.php (2)

74-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Scope the text-white negative assertion.

Line 74 (not->toContain('text-white')) is page-global and can fail from unrelated components. Limit this check to the social-links block/anchor classes under test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app-modules/portal/tests/Feature/SocialLinksPageTest.php` at line 74, The
assertion on line 74 checks that 'text-white' does not appear anywhere on the
entire page, which is too broad and can fail due to unrelated components.
Instead of asserting against the full $html, scope the check to only the
social-links block or container element being tested. Extract or identify the
relevant portion of the HTML (such as the social-links div or anchor elements)
and apply the not->toContain('text-white') assertion only to that scoped section
to ensure the test only validates the social-links component under test.

49-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid full inline-style string matching.

Lines 49-50 depend on exact whitespace/order in style="". Assert tokens independently (--accent-light: #... and --accent-dark: #...) to prevent fragile failures.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app-modules/portal/tests/Feature/SocialLinksPageTest.php` around lines 49 -
50, The test assertions in SocialLinksPageTest.php at lines 49-50 are checking
for exact inline style strings which makes the test brittle to whitespace or
property order changes. Instead of using toContain to match the full style
string '--accent-light: `#0F172A`; --accent-dark: `#FFFFFF`;', split these
assertions to independently verify that the HTML contains the individual CSS
custom property tokens (--accent-light: `#0F172A` and --accent-dark: `#FFFFFF`
separately) so the test only fails if the actual CSS values change, not the
formatting.
app-modules/portal/tests/Feature/HomepageTest.php (1)

27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Harden the dark-class assertion on <html>.

Line 27 checks one exact HTML string and can pass even if dark is still present with different attribute ordering/class composition. Assert absence of dark on the <html> tag via a scoped regex/DOM parse instead of exact serialization.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app-modules/portal/tests/Feature/HomepageTest.php` at line 27, The assertion
in HomepageTest.php currently checks for an exact HTML string match using
toContain() which is fragile and will fail to catch the dark class if attributes
are reordered or classes are combined differently. Replace the exact string
check with a more robust regex pattern that matches the html tag opening
regardless of attribute order, or alternatively parse the HTML using DOM methods
to specifically assert that the html element does not contain the dark class.
This will ensure the assertion reliably verifies the absence of the dark class
on the html tag regardless of how the HTML is formatted or what other classes or
attributes are present.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app-modules/portal/tests/Feature/HomepageTest.php`:
- Line 27: The assertion in HomepageTest.php currently checks for an exact HTML
string match using toContain() which is fragile and will fail to catch the dark
class if attributes are reordered or classes are combined differently. Replace
the exact string check with a more robust regex pattern that matches the html
tag opening regardless of attribute order, or alternatively parse the HTML using
DOM methods to specifically assert that the html element does not contain the
dark class. This will ensure the assertion reliably verifies the absence of the
dark class on the html tag regardless of how the HTML is formatted or what other
classes or attributes are present.

In `@app-modules/portal/tests/Feature/SocialLinksPageTest.php`:
- Line 74: The assertion on line 74 checks that 'text-white' does not appear
anywhere on the entire page, which is too broad and can fail due to unrelated
components. Instead of asserting against the full $html, scope the check to only
the social-links block or container element being tested. Extract or identify
the relevant portion of the HTML (such as the social-links div or anchor
elements) and apply the not->toContain('text-white') assertion only to that
scoped section to ensure the test only validates the social-links component
under test.
- Around line 49-50: The test assertions in SocialLinksPageTest.php at lines
49-50 are checking for exact inline style strings which makes the test brittle
to whitespace or property order changes. Instead of using toContain to match the
full style string '--accent-light: `#0F172A`; --accent-dark: `#FFFFFF`;', split
these assertions to independently verify that the HTML contains the individual
CSS custom property tokens (--accent-light: `#0F172A` and --accent-dark: `#FFFFFF`
separately) so the test only fails if the actual CSS values change, not the
formatting.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro

Run ID: ee1f96a9-420e-44be-b47f-eb9897f1c7a2

📥 Commits

Reviewing files that changed from the base of the PR and between ade5d6c and 7f79fec.

📒 Files selected for processing (9)
  • app-modules/he4rt/resources/css/components/button.css
  • app-modules/portal/resources/views/components/layouts/app.blade.php
  • app-modules/portal/resources/views/components/logo.blade.php
  • app-modules/portal/resources/views/components/navbar.blade.php
  • app-modules/portal/resources/views/social-links.blade.php
  • app-modules/portal/src/DTOs/SocialLink.php
  • app-modules/portal/src/Livewire/SocialLinksPage.php
  • app-modules/portal/tests/Feature/HomepageTest.php
  • app-modules/portal/tests/Feature/SocialLinksPageTest.php

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@fernanduandrade You're correct — this PR only modified the SocialLink constructor (added the optional accentDark param) and updated the links() method body. No new methods were introduced. The docstring coverage failure is pre-existing and not caused by changes in this PR.


✏️ Learnings added
Learnt from: fernanduandrade
URL: https://github.com/he4rt/heartdevs.com/pull/371

Timestamp: 2026-06-24T16:24:40.033Z
Learning: In the he4rt/heartdevs.com repository, the Docstring Coverage pre-merge check failure (50% coverage vs 80% threshold) is a pre-existing issue, not introduced by individual PRs. Contributors should not be blocked on this check when their PR does not add new methods.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

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.

Light Theme Compatibility Fix for Routes Missing Proper Theming Support

2 participants