Skip to content

fix multiple ui and content issues#377

Draft
mostafaNazari702 wants to merge 1 commit intoReVanced:devfrom
mostafaNazari702:dev
Draft

fix multiple ui and content issues#377
mostafaNazari702 wants to merge 1 commit intoReVanced:devfrom
mostafaNazari702:dev

Conversation

@mostafaNazari702
Copy link
Copy Markdown
Contributor

No description provided.

…update deps

fix(announcements): fix hydration mismatch and FOUC on card content

The ReVanced API returns announcement content as rich HTML, including
malformed tags, embedded <style> blocks, and unquoted attributes.
Using {@html} during SSR caused the browser to restructure the DOM to
correct these issues, producing a tree that no longer matched Svelte's
expected hydration structure, resulting in:

DOMException: Node.appendChild: Cannot add children to a Text

In AnnouncementCard, replace {@html} with a plain-text derived value
that strips HTML tags. This renders correctly on the server, eliminating
both the hydration mismatch and the flash of empty content that appeared
when the client-only renderHtml action was used as an interim fix.

In Content (the detail view), replace {@html} with a use:renderHtml
action that sets innerHTML client-side after hydration, safely rendering
full rich HTML without a hydration conflict. The flash is acceptable
here since this is a client-navigated detail panel, not SSR-critical.

Additional changes in this commit:

- fix(tooltip): rewrite ToolTip component using position:fixed and
  JS-driven positioning so tooltips on archived announcement cards
  are never clipped by overflow:hidden containers or viewport edges.
  Add max-width, multi-line wrapping, and centered text for readability.

- fix(announcements): separate tag filters for active and archived
  sections. Active tags use URL-synced state; archived tags use
  independent local state. Clicking a tag in one section no longer
  affects the other.

- fix(announcements): tags belonging only to archived announcements
  now appear exclusively inside the archive section (visible when
  expanded). Shared tags appear in both sections.

- fix(announcements): strip <style> and <script> blocks (including
  their inner text) from HTML content before tag-stripping, so CSS
  rules no longer leak into card preview text.

- fix(announcements): remove justify-content:space-between from card
  layout to eliminate the empty gap between timestamp and content.
  Tags are pinned to the card bottom via margin-top:auto instead.

- fix(api): make 'status' field optional in AboutSchema and About
  type so upstream API responses (which omit it) no longer cause
  Zod validation failures on every page load.

- fix(api/server): correct endpoint typo 'announcement/latest' to
  'announcements/latest'.

- fix(layout): access created_at via item.announcement.created_at
  instead of item.created_at in the layout banner filter.

- fix(announcements): always send tags array in announcement update
  payload so clearing all tags is persisted correctly.

- chore(deps): update project dependencies
transform: translateX(-50%);
margin-bottom: 8px;
padding: 0.75rem 1rem;
/* Fixed escapes overflow:hidden on any ancestor (e.g. archive collapse container)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I dont think this comment is supposed to be here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was certain that i needed to explain the big changes in the file and the removal of many css properties but will be removed.

announcement.archived_at ? relativeTime(announcement.archived_at) : ''
);

let textContent = $derived(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Whats this for

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so the card only shows the actual readable text instead of hidden website formatting/code from the announcement card in the announcements page.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Before i fixed it, it was like this on the card:
li { font-size: initial; font-weight: inherit; } pre { overflow: auto; padding: 1rem; border-radius: 0.25rem; background: var(--surface-four); }

@oSumAtrIX oSumAtrIX requested a review from madkarmaa March 26, 2026 19:26
Copy link
Copy Markdown
Member

@Ushie Ushie left a comment

Choose a reason for hiding this comment

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

Forgot to submit my reviews, gg

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use a package like SVooltip, a custom ToolTip implementation is not necessary

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

SVooltip do not support interactive tooltips (hovering into the tooltip to click links/buttons inside it). From their own docs it says: "Including support for hovering the tooltip and clicking buttons inside and all that jazz. This will give you a tooltip. That's it."

We use interactive mode in TeamMemberCard to show a GPG key ID as a clickable link inside the tooltip. SVooltip cannot handle this, if my knowledge is at its peak. It also doesn't support svelte snippets for structured tooltip comtent.

The custom component is around 90 lines of logic + CSS, uses position: fixed´ to escape ancestor overflow:hidden` (which was the original bug), and integrates with the project's CSS variables. It's small and purpose-built for exactly our requirements.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You dont need the GPG Key ID to be a clickable link, the icon itself should be clickable, as it was pre-rewrite

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

but it doesnt hurt to have the id to be clickabe too

Copy link
Copy Markdown
Member

@Ushie Ushie Mar 26, 2026

Choose a reason for hiding this comment

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

With this much code and pretty much no real usecase I'd rather use Svooltip

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You dont need the GPG Key ID to be a clickable link, the icon itself should be clickable, as it was pre-rewrite

TeamMemberCard.svelte has zero changes in this PR. the interactive tooltip with the clickable GPG key link existed before this PR in the rewrite, identically. This PR only changed the ToolTip's positioning from position: absolute to position: fixed + JS to fix tooltips being clipped by overflow:hidden on the archive collapse container.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

With this much code and pretty much no real usecase I'd rather use Svooltip

The code increase in this PR is the JS positioning logic (80 lines~) that fixes tooltips being clipped inside the archive collapse container (overflow:hidden). The old position: absolute approach couldn't escape ancestor clipping, that was the bug.

Adopting SVooltip is a separate discussion from this bugfix PR. It would require removing the interactive tooltip that was designed in the rewrite, refactoring 3 cosumer components, and adding a new dependency. Happy to discuss that as a separate issue if (OsumAtrix/$) wants to evaluate it, but bunndling it into this bugfix would expand the scope significantlly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's fine I will do it myself later

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's fine I will do it myself later

You can do that but i, from my experience, can assure you that you will get a ton of bugs. The code will also become significantly more to fix each bug individually. I do however wish you best of luck!

Comment on lines +30 to +39
let textContent = $derived(
announcement.content
? announcement.content
.replace(/<style[^>]*>[\s\S]*?<\/style>/gi, '')
.replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '')
.replace(/<[^>]*>/g, ' ')
.replace(/\s+/g, ' ')
.trim()
: ''
);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What issue does this solve? Styling was intended for the preview, the CSS should be adjusted for it not to be as jarring

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