Skip to content

Fix and adjust alert margins#86

Merged
Shiroizu merged 2 commits intoVocaDB:mainfrom
Hans5958:layout/alert-margin-fix
Jan 5, 2026
Merged

Fix and adjust alert margins#86
Shiroizu merged 2 commits intoVocaDB:mainfrom
Hans5958:layout/alert-margin-fix

Conversation

@Hans5958
Copy link
Copy Markdown
Contributor

This PR fixes the missing margin on the "Prioritize official translations" rule page (see example), as well as adjusting the margins on the alert, primarily to reduce it between the alert title and its description.

Preview

Before

After

Prettier wants to collapse it even though I wanted a paragraph out of it.
@Hans5958 Hans5958 force-pushed the layout/alert-margin-fix branch from 9a0b153 to 8c8a937 Compare December 27, 2025 15:32
Copy link
Copy Markdown
Member

@Shiroizu Shiroizu left a comment

Choose a reason for hiding this comment

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

Is there a reason for including <></> inside the Alerts ?

@Hans5958
Copy link
Copy Markdown
Contributor Author

Hans5958 commented Jan 5, 2026

Prettier wants to collapse it even though I wanted a paragraph out of it.

If I do this:

<Alert>
	**English**: Servant of Evil
</Alert>

It will do a paragraph, which is semantically correct and produces the margin (since margins are on the <p>).

<div>
	<p>Incorrect</p>
	<div>
		<p>
			<strong>English</strong>: Servant of Evil
		</p>
	</div>
</div>

But Prettier really wants this:

<Alert>**English**: Servant of Evil</Alert>

Which will cause this and makes the margin missing, since the <p> is missing.

<div>
	<p>Incorrect</p>
	<div>
		<strong>English</strong>: Servant of Evil
	</div>
</div>

This is why I did the <></> workaround so that Prettier won't collapse it.

<Alert>
	<></>
	**English**: Servant of Evil
</Alert>

@Shiroizu
Copy link
Copy Markdown
Member

Shiroizu commented Jan 5, 2026

Maybe this pattern could be extracted to a separate component:

 <Alert variant="incorrect">
    <></>
    **English**: Servant of Evil
  </Alert>
  <Alert variant="correct">
    <></>
    **English**: His significance of existence
  </Alert>

-->

<Incorrect line={"English: Servant of Evil"} />
<Correct line={"English: His significance of existence"} />

This way we could also avoid the extra markers <></> and fix it in a single location (with prettierignore or something).

@Hans5958
Copy link
Copy Markdown
Contributor Author

Hans5958 commented Jan 5, 2026

Such addition would introduce unnecessary complexity that is not worth the time maintaining. The <Alert> component is supposed to be simple enough to be versatile, so I don't really want to implement that to avoid one Prettier edge case.

Note that the component has more use, such as:

  <Alert variant="incorrect">
    <table class="table-fixed">
      <tr>
        <th>Non-English</th>
        <td>Machine Love</td>
      </tr>
      <tr>
        <th>Romanized</th>
        <td>Machine Love</td>
      </tr>
      <tr>
        <th>English</th>
        <td>Machine Love</td>
      </tr>
    </table>
  </Alert>

So having the line argument doesn't really work, or worth the time to maintain. There could be an argument of doing <p> if there's only one line, but that's still quite a fluff and not dealing with the root cause (I might do this, or just adding <p> on the usage to force it).

Adding separate exported components (<Correct>, <Incorrect>) also just adding more lines that have small value (since you have the variant argument to use).

@Shiroizu Shiroizu merged commit 979397d into VocaDB:main Jan 5, 2026
1 check passed
@Hans5958 Hans5958 deleted the layout/alert-margin-fix branch January 7, 2026 11:05
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.

2 participants