Skip to content

feat: support overriding TopBar #112

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

Merged
merged 6 commits into from
Jul 9, 2024

Conversation

AriPerkkio
Copy link
Member

@AriPerkkio AriPerkkio commented Jul 3, 2024

Adds support for overriding TopBar component. In future we can add support for more component overrides.

import tutorialkit from '@tutorialkit/astro';
import { defineConfig } from 'astro/config';

export default defineConfig({
  integrations: [
    tutorialkit({
      components: {
        TopBar: './CustomTopBar.astro',
      },
    }),
  ],
});

Users may still want to utilize TutorialKit's default <Logo> and <ThemeSwitch>. These are accessible through slots:

// ./CustomTopBar.astro
<nav style="display: flex; align-items: center; gap: 2rem; padding: 1rem;">
  Some content here
  <slot name="logo" />
  and here
  <slot name="theme-switch" />
  here too
  <slot name="login-button" />
  and here!
</nav>

Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Comment on lines +1 to +4
---
title: 'Overriding Components'
description: "Override TutorialKit's default components to fit your needs."
---
Copy link
Member Author

Choose a reason for hiding this comment

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

These docs are highly inspired by Starlight: https://starlight.astro.build/guides/overriding-components/

@AriPerkkio AriPerkkio force-pushed the feat/override-components branch 2 times, most recently from 7141020 to 9700943 Compare July 4, 2024 13:33
@AriPerkkio AriPerkkio marked this pull request as ready for review July 4, 2024 13:45
@AriPerkkio
Copy link
Member Author

I spent some time trying to set up tests for this kind of features but it seemed a bit bigger task than I expected. I don't think there are any existing tests where this feature could be easily tested..? Let's build good testing setup in separate PR later.

Next I'll look into adding overrides for other components so that rest of the feature requests can be filled. For example the bottom-left corners "Edit this page" link.

@AriPerkkio AriPerkkio requested review from Nemikolh and d3lm July 4, 2024 13:49
@Nemikolh
Copy link
Member

Nemikolh commented Jul 4, 2024

Let's build good testing setup in separate PR later.

I totally agree. In particular I think this would be easy to test if we had playwright or puppeteer configured.

@AriPerkkio
Copy link
Member Author

I totally agree. In particular I think this would be easy to test if we had playwright or puppeteer configured.

Yep! I had something like this in mind:

import { fileURLToPath } from "node:url";
import { beforeAll, expect, test } from "vitest";
import { type Browser, chromium } from "playwright";
import { dev } from "astro";

let browser: Browser;

beforeAll(async () => {
  // Start server
  const server = await dev({
    configFile: false,
    root: fileURLToPath(new URL("./fixtures", import.meta.url)),
    integrations: [
      tutorialkit({
        components: {
          TopBar: "./src/components/CustomTopBar.astro",
        },
      }),
    ],
  });

  // Open browser
  browser = await chromium.launch({ headless: true });

  // Close server
  return async function cleanup() {
    await server.stop();
  };
});

test("custom top bar is visible", async () => {
  const page = await browser.newPage();
  await page.goto("http://localhost:4321/");

  await page.getByRole("heading", {
    level: 1,
    name: "Custom top bar",
  });
});

But TutorialKit had some issues when used from Astro's Node API and changing root.

Copy link
Member

@Nemikolh Nemikolh left a comment

Choose a reason for hiding this comment

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

Looks pretty good and the docs are top notch!! 🤩

Left a few comments, the more important being to consider the impact of doing an eject.

@AriPerkkio AriPerkkio force-pushed the feat/override-components branch from 6267cde to f955653 Compare July 9, 2024 10:44
@AriPerkkio AriPerkkio force-pushed the feat/override-components branch from f955653 to 01dd492 Compare July 9, 2024 10:46
Copy link
Contributor

@d3lm d3lm left a comment

Choose a reason for hiding this comment

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

Nice one!

@AriPerkkio AriPerkkio force-pushed the feat/override-components branch 4 times, most recently from bf47282 to ce0a6ec Compare July 9, 2024 11:47
@AriPerkkio AriPerkkio force-pushed the feat/override-components branch from ce0a6ec to c73dcf5 Compare July 9, 2024 11:54
@AriPerkkio
Copy link
Member Author

Tests on Windows CI were failing but it's fixed now. ✅

Copy link
Member

@Nemikolh Nemikolh left a comment

Choose a reason for hiding this comment

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

Nice, really good work on this!! 🤩

@Nemikolh Nemikolh merged commit 3792aa9 into stackblitz:main Jul 9, 2024
8 checks passed
@AriPerkkio AriPerkkio deleted the feat/override-components branch July 9, 2024 14:07
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