Skip to content
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

feat: adds the ability to view draft posts in dev mode #223

Closed
wants to merge 4 commits into from

Conversation

SSmale
Copy link
Contributor

@SSmale SSmale commented Jan 8, 2024

Currently you need to make a post not in draft (set the frontmatter key draft to false to get it to render. This makes it easy to accidently commit a draft post with the value set to false when you want to see how it will render.

This change allows posts marked as draft to be seen when the builtin astro env variable is set to development (when it is run with astro dev or npm start for us).

When the app is built (astro build) the env variable will be production and the usual filter is set.

https://docs.astro.build/en/guides/environment-variables/#default-environment-variables

@satnaing
Copy link
Owner

satnaing commented Jan 10, 2024

Great idea.
One thing about this is that some users might see this confusing even though titles of draft posts in DEV mode start with the word "DRAFT: ". Better approach for this, in my opinion, is giving users some kind of toggle during DEV mode.
Maybe AstroPaper could make use of Astro v4 new Dev Toolbar integration for this kind of feature. 💡
I haven't dig the API enough and thus not sure whether it is possible.
But if it is, it will be beneficial and more convenient for users.

can be defaulted to true by running `DRAFT_MODE=true npm start`
@SSmale
Copy link
Contributor Author

SSmale commented Jan 10, 2024

Thanks for the pointer, it is done. I have also posted in the Astro Docs discussion as the docs are hard to follow on the feature

Screen.Recording.2024-01-10.at.22.19.40.mov

Not sure where you want to document the feature. it is off by default but can be set to true by having an env var of DRAFT_MODE=true either in an env file or as part of the run command DRAFT_MODE=true npm start

@satnaing
Copy link
Owner

Awesome!
I was also experimenting the Devtool api by myself.
We have similarities in some area, but some approach are different.
You are using localStorage for the draft and I think it is the right approach.
I was at first doing a attribute approach.
Yours seems to be a better approach compared to mine (except the TypeScript part).

Here are my changes I made 2 days ago.

file: src/packages/draft-toolbar/integrations.ts

import type { AstroIntegration } from "astro";
import path from "path";
import url from "url";

export default (): import("astro").AstroIntegration => ({
  name: "draftToolbar",
  hooks: {
    "astro:config:setup": ({ addDevToolbarApp }) => {
      const importPath = path.dirname(url.fileURLToPath(import.meta.url));
      const pluginPath = path.join(importPath, "my-app.ts");
      addDevToolbarApp(pluginPath);
    },
  },
});

file: src/packages/draft-toolbar/my-app.ts

import type { DevOverlayPlugin } from "astro";

export default {
  id: "paper-dev-tool",
  name: "Paper DevTool",
  icon: `
  <svg xmlns="http://www.w3.org/2000/svg" class="icon icon-tabler icon-tabler-file-code" width="24" height="24" viewBox="0 0 24 24" stroke-width="2" stroke="currentColor" fill="none" stroke-linecap="round" stroke-linejoin="round"><path stroke="none" d="M0 0h24v24H0z" fill="none"/><path d="M14 3v4a1 1 0 0 0 1 1h4" /><path d="M17 21h-10a2 2 0 0 1 -2 -2v-14a2 2 0 0 1 2 -2h7l5 5v11a2 2 0 0 1 -2 2z" /><path d="M10 13l-1 2l1 2" /><path d="M14 13l1 2l-1 2" /></svg>
  `,
  init(canvas, eventTarget) {
    canvas.appendChild(document.createTextNode("Hello World!"));
    // canvas.

    // console.log(canvas.appendChild);
    eventTarget.dispatchEvent(
      new CustomEvent("app-notification", {
        detail: {
          state: true,
        },
      })
    );
    eventTarget.addEventListener("app-toggled", event => {
      if ((event as CustomEvent).detail.state === true) {
        console.log("The app is now enabled!");
        document.body.setAttribute("data-show-draft", "true");
      } else {
        document.body.setAttribute("data-show-draft", "false");
      }
    });
  },
} satisfies DevOverlayPlugin;

file: astro.config.ts

import { defineConfig } from "astro/config";
import tailwind from "@astrojs/tailwind";
import react from "@astrojs/react";
import remarkToc from "remark-toc";
import remarkCollapse from "remark-collapse";
import sitemap from "@astrojs/sitemap";
import { SITE } from "./src/config";
import draftToolbar from "./src/packages/draft-toolbar/integrations";

// https://astro.build/config
export default defineConfig({
  site: SITE.website,
  integrations: [
    tailwind({
      applyBaseStyles: false,
    }),
    react(),
    sitemap(),
    draftToolbar(),
  ],
  markdown: {
    remarkPlugins: [
      remarkToc,
      [
        remarkCollapse,
        {
          test: "Table of contents",
        },
      ],
    ],
    shikiConfig: {
      theme: "one-dark-pro",
      wrap: true,
    },
  },
  vite: {
    optimizeDeps: {
      exclude: ["@resvg/resvg-js"],
    },
  },
  scopedStyleStrategy: "where",
});

I'm just thinking that AstroPaper devtool could be a separate npm package rather than sitting inside the template.
This way, users who want to integrate the devtool can opt-in the package while others can choose not to do so.
Besides, it has much more potential than draft functionality; like theme options, meta data etc.
What do you think?

@SSmale
Copy link
Contributor Author

SSmale commented Jan 12, 2024

It has been a while since I have used ts and things have moved on a little, also Astro is all new to me to I went with js to keep it simple.

There are the canvas elements so you can build a control panel for more functions, which would be cool for the other parts. Just not sure what other things you'd want to toggle as it is only there in dev mode, I am not sure the use cases they imagined.

I think it is already opt in by basically being hidden. It is going to be quite tightly coupled to AstroPaper so might not be much use standalone. Would you be able to use it on another theme? Do all Astro based blogs have the same front matter? I am one to keep things monolithic until there is a reason to break them out. I do get the icons and GitLab as blog posts but they are all in the one repo so you can check it out and have everything you need.

Something that might need to go in before it is released is a way to suppress the logs. They do get noisy but are useful to see how it works.

@satnaing
Copy link
Owner

It has been a while since I have used ts and things have moved on a little, also Astro is all new to me to I went with js to keep it simple.

Yep, understandable!
I can co-author for that tho.

There are the canvas elements so you can build a control panel for more functions, which would be cool for the other parts. Just not sure what other things you'd want to toggle as it is only there in dev mode, I am not sure the use cases they imagined.

I'm thinking something like social images and meta tags previews in dev mode. This could be beneficial and I've been debugging social images and meta tags using chrome extensions.
And maybe some config options inside the src/config.ts 🤷

I think it is already opt in by basically being hidden. It is going to be quite tightly coupled to AstroPaper so might not be much use standalone. Would you be able to use it on another theme? Do all Astro based blogs have the same front matter? I am one to keep things monolithic until there is a reason to break them out. I do get the icons and GitLab as blog posts but they are all in the one repo so you can check it out and have everything you need.

This might be bias (maybe childish XD) but I wanted to publish a package since I've never done one. 😃
Also, there's a plan to add a CLI tool for AstroPaper, like scaffolding, adding extra integrations (features) etc.
Just like shadcn/ui, AstroPaper users will be able to add integrations on their own by simply installing that ingetration.
For example, currently, if a user want to add a reading time in their post, they have to follow along the blog post. But making a separate package would allow them to get this functionality by just running a command in their terminal.

@SSmale
Copy link
Contributor Author

SSmale commented Jan 13, 2024

Please feel free to update the code.

Yeah that could be good, I think though if you want it to be an optional NPM package there will be some refactoring needed to decouple those parts, currently there are a lot of places that need changing, you would need some kind of hook system for the rpm package to override it.

if you are looking to add these tools, it might be worth setting up an astropaper GitHub org to group them altogether.

@satnaing
Copy link
Owner

@SSmale

if you are looking to add these tools, it might be worth setting up an astropaper GitHub org to group them altogether.

Yep. I'll consider doing so when I add more related integrations etc. If so, might need help since never done anything like that before.

For now, I'll merge it after I made a few minor changes.

Changes made
- transform JavaScript files into TypeScript
- remove unnecessary @utils/draftFilter codes
- move toolbar/draftMode into packages/toolbar/draft-mode
- rename utils/postFilters.ts to utils/postFilter.ts
- rename draftFilter to postFilter in utils/postFilter.ts
@satnaing
Copy link
Owner

Hello @SSmale
I made a few changes and you can also give your opinion on that.
If we're satisfied with the result, we can proceed to resolve the conflicts.

@SSmale
Copy link
Contributor Author

SSmale commented Jan 16, 2024

That all looks good to me.

@satnaing
Copy link
Owner

I noticed some bugs.

419438130_7647703271915189_4958357625939483701_n.mp4

Also, in Firefox, there's a refresh bug which looks like an infinite loop in React.

419472324_7417410228309245_2100579450384296129_n.mp4

@SSmale
Copy link
Contributor Author

SSmale commented Jan 17, 2024

I had the infinite loop. It was due to the blocks not working. I think there was something about needing to restart the server after a change in writing the code as it didn't pick up one of the changes.

@satnaing
Copy link
Owner

satnaing commented Jan 18, 2024

@SSmale
I think I found a way to solve this problem.
But it is a complete different approach.
I'll refactor, test and post it tmr since it's almost midnight here.

@SSmale
Copy link
Contributor Author

SSmale commented Jan 18, 2024

Ok. Interested to see the changes :)

@satnaing
Copy link
Owner

satnaing commented Jan 20, 2024

I've solved that issue using client side config and css only.
But I'm not satisfied with the result and I think it's still not the right approach.

Here's the result.

420996733_24536330532678497_1519035981881077858_n.mp4

@SSmale
Copy link
Contributor Author

SSmale commented Jan 20, 2024

I’m not sure what that video is showing?

Is it worth backing the changes out and testing my original code again, then slowly add the TS in, then move it to the package structure?

@SSmale
Copy link
Contributor Author

SSmale commented Jan 20, 2024

reload loop

Just tested it with my original PR code, Safari works fine, but get the reload loop on chrome and Firefox.

Looks like the issue is having more than one instance (window or tab) open as they are each fighting to set the flag

404 error

Interestingly, if you add the following to src/pages/posts/[slug]/index.astro in the getStaticPaths() function, they no longer 404

  console.log(
    posts.map(x => {
      return {
        title: x.data.title,
        draft: x.data.draft,
      };
    })
  );

@satnaing
Copy link
Owner

@SSmale
The video is about toggling draft mode; and as you can see, the post DRAFT:AstroPaper 4.0 is hidden and reappeared as we toggle the dev toolbar.
This is done by only using CSS.
But it has some trade-offs. For example, it only hides the draft posts visually. It doesn't actually remove from the DOM. Paginations can be incorrect and confusing by using this way.

@satnaing
Copy link
Owner

The best way to handle this, in my opinion, is sending events to postFilter.ts file when the toolbar is toggled.
I can detect dev toolbar toggling inside integration.ts file.

      server.ws.on("astro-dev-toolbar:paper-dev-tool:toggled", data => {
          // can detect event inside this
      });

The thing is... that event cannot be detected inside the postFilter function.
I've tried server.ws.send and it's not working as expected.
I've also checked and tried Vite HMR API but no progress. Maybe because I'm not familiar with that API 🤷.

@satnaing
Copy link
Owner

The main issue with this feature is that we can't directly control servercode using the Dev Toolbar API. While it enables communication between the client and server during rendering, it's not seamless with HMR.

This leads us to rely on JavaScript and DOM manipulation instead of server draft filtering, which affects DX. For example, enabling draft mode can sometimes cause 404 errors or infinite loops.

463099210_8597357733658839_4797609766704187343_n.mp4

Given these challenges, I won’t merge this feature for now. However, I’ll definitely consider adding it once a better approach is found.

Thanks again for your contributions and discussions, @SSmale!

@satnaing satnaing closed this Oct 13, 2024
@SSmale
Copy link
Contributor Author

SSmale commented Oct 13, 2024

Thanks for the update, is it worth going back to the original always on in dev mode in the interim?

@satnaing
Copy link
Owner

I don't quite get what you mean @SSmale

@SSmale
Copy link
Contributor Author

SSmale commented Oct 13, 2024

The original PR was for it to show draft posts with a custom title prefix when the app was running in dev mode.

@satnaing
Copy link
Owner

The loop issue I mentioned still exists in the original PR.
You can test it by checking out this commit.
If we open the two tabs at the same time, for some reason, the infinite loop begins.
So, for now, I'll focus on other issues and features.
However, thanks for this PR and your previous contributions. ✨

@SSmale
Copy link
Contributor Author

SSmale commented Oct 14, 2024

The loop issue I mentioned still exists in the original PR.

You can test it by checking out this commit.

There were two approaches. My original one here, where I am just changing the filter criteria based on the mode, rather than using the Astro dev tool bar. The tool bar approach was when the loop was introduced.

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