vuejs / vitepress

Vite & Vue powered static site generator.
https://vitepress.dev
MIT License
11.48k stars 1.86k forks source link

feat(default-theme): native support for carbon ads #86

Closed posva closed 3 years ago

posva commented 3 years ago

I also added the existing slots sidebar-top/bottom and page-top/bottom because I used them to add the ads components. To make them work and test them, you need to add this config to the themeConfig:

    carbonAds: {
      carbon: 'CEBICK3I',
      custom: 'CEBICK3M',
      placement: 'routervuejsorg',
    },
kiaking commented 3 years ago

I kinda like the idea of having Google Analytics option, since usually everybody wants that. But not sure about the ads... Because while Vue's official docs uses Carbon, it really depends on locales what the primary ads formats are. I think for now, ads should be configured for each repo (Router/Vuex/etc.) using slots 🤔 What do you think? (Again, I'm all for Google Analytics though).

posva commented 3 years ago

I think we should still add the slots to the default theme and then add configs for ads to the vue theme since we use it in every official vue lib documentation. We could also handle something per locale, and it could also be a plugin instead.

Adding analytics should also be in a plugin like in vuepress.

But I don't think we have the plugin architecture yet

kiaking commented 3 years ago

Ah, that make sense. If we were to implement plugins feature (and I think we kinda need it), it make sense now to include these and pull it out once we have the plugin feature 👍

posva commented 3 years ago

yeah, let's hold on to this for the moment. I will refactor it once there is some kind of plugin architecture in place

kiaking commented 3 years ago

@posva I think at the moment, VitePress doesn't look like to have any strong plugin feature implemented in future. I think we should have this merged in to the core directly.

posva commented 3 years ago

@kiaking I think it's fine to wait for the moment! Better have a plugin architecture built that allows adding such things. Do you want to add this feature for Vuex or something? The current PR is not up to date with what exists in vue router next either. I would have to update it. I even plan on changing what I currently have on Vue Router docs to allocate spots for sponsors.

kiaking commented 3 years ago

I think it's fine to wait for the moment! Better have a plugin architecture built that allows adding such things.

No I mean the discussion we are having right now, there might be no plugin feature for VitePress at all. So we might be waiting for nothing... 🤔

Do you want to add this feature for Vuex or something?

This is another story but yes I would love to. And for that, I can of course wait!

posva commented 3 years ago

I will give this a go and refactor next week and make it look like the current version on vue router next docs: https://next.router.vuejs.org/guide/essentials/navigation.html#navigate-to-a-different-location which was inspired by vuejs.org

kiaking commented 3 years ago

Yeah great idea! Looking forward to it! I might be modifying the components a lot for stylings and refactoring. So don't worry so much about conflicts, I think I can resolve those.

posva commented 3 years ago

Here are the changes needed. The overflow-x: auto fixes this:

Screen Shot 2020-11-23 at 12 19 29

into

Screen Shot 2020-11-23 at 12 43 52

Note you will have to add that style rule to .scrimba for vuex docs

posva commented 3 years ago

I think we should add by default the algolia search too, but I will do that in a separat PR. What do you think @kiaking ?

kiaking commented 3 years ago

Yes we should definitely have algolia integration too, the issue #40 🙌

posva commented 3 years ago

ah yeah, that's a different thing though 😆 : simple search out of the box based on titles. Algolia still needs you to request ids at https://docsearch.algolia.com/

posva commented 3 years ago

This should be ready for review!

kiaking commented 3 years ago

Here we go!