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

FAQ Plugin #116

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

FAQ Plugin #116

wants to merge 33 commits into from

Conversation

sustained
Copy link
Contributor

@sustained sustained commented Dec 29, 2019

NOTE

This looks like a big PR but bear in mind that 60 FAQ entries have been split into separate files, so don't worry too much.

What does this PR do?

In short:

  • each FAQ section/category is now a directory
  • each FAQ item is now its own file
  • there is a link to edit each file on the FAQs page
Screenshots

image

image

image

Why would we want this?

Allow looking up of FAQ data via Vue Land Discord bot.

The primary motivation for me personally is so that the up and coming official Vue Land bot will be able to make use of this data (i.e. we'll be able to look up the FAQs from the Discord server via !faq <query>). I tried messing around with Markdown parse trees and such and it was rather a pain, much more work than this PR afaict.

Easier maintenance of FAQs going forward?

Regardless, I think that having most of the data in the YAML Front Matter in this way makes life easier, should we ever need to add more info./data to FAQs, or change the structure en masse?

Also, it's kind of easier for contributors to work with this instead of one gargantuan markdown file, don't you think? Maybe not but I think so. 😓

🆕 Allows sorting/filtering of FAQ items.

We could provide filtering by e.g.:

  • difficulty - beginner, intermediate, advanced
  • frequency (how often the issue shows up in the real world) - common, uncommon
  • whatever else our hearts desire

Issues with the previous iteration

At first I tried making it so that the FAQ pages were normal VuePress pages and so I dug into the docs, and into the source code but just couldn't figure out how to make that work in a nice way, due to several problems:

  1. It's not possible to pass along the actual markdown content, parsed or unparsed, without parsing it a second time ourselves and passing that along... ugh.
  2. It's not possible to disable route generation for certain pages, therefore pages that we don't want to have routes associated with them would.

Issues with the current implementation

I think this implementation is a fair bit cleaner than when I tried it the other way, honestly, though it does present its own problems:

  1. The categories are actually specified in the YAML Front Matter, the directories are just for ordering and organisational purposes. The issue here being case-sensitivity - Vue is not the same category as vue. Is this really a problem? If so it's fairly easy to fix.
  2. There's currently no way to attach metadata to the categories (e.g a description), so it's not possible to display some text under each category heading, for instance. Do we need that? It wouldn't be hard to add.
  3. The ordering of both categories and questions is defined by the file/directory name (so 1-vue displays before 2-components). I don't personally see this as a problem but perhaps you do? The alternative is I guess it being in the Front Matter? Personally I find that inferior.
  4. I ran into an odd CSS issue which is documented in a comment in the <style> block of this file where the header is hidden under the navigation. I managed to fix it but the fix feels hacky. Anyone know what is going on?
  5. We can't use components anymore inside the files. I just discovered this one but it should have been obvious in hind sight. This is potentially quite a big issue... for now it only means that badges don't work. However we can still use markdown extensions - is creating a markdown extension for badges an acceptable compromise?

Anything else?

I suppose, in a way, this relates also to #99 and it could be nice to extract this out into a more reusable plugin that supports arbitrary collections of data (one for FAQs, one for libraries, etc.). I'll have to have a think about how that might be achieved.


I can totally understand if you don't want to merge, maybe this code isn't my proudest moment 😁 but I tried my best to clean it up as best I could and document how to use it before sending in the PR.

Feedback, as always, welcome.f

plugins/faqs/index.js Outdated Show resolved Hide resolved
@sustained
Copy link
Contributor Author

See also this issue on VuePress as basically that's the feature we want/need. Depending on their response, I may end up making this into some kind of VuePress plugin that we can use for more than FAQs, if we can't get such a feature (aka Jekyll collections) into VuePress core.

Still, I just wanted to see how this might look if I attempted to implement it, call it an experiment if you want. At least some of the work on this PR would still be useful (i.e. the extracting of the FAQ items) if we go the other route.

Copy link
Owner

@dobromir-hristov dobromir-hristov left a comment

Choose a reason for hiding this comment

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

Hey.
First I want to say thank you, for probably spending allot of time.
Now looking at this, many questions rise in my head, so we do this little by little.

  1. You said parsing the MD FAQ file was harder than this whole PR? Couldnt we have had an export task, that would just scan that file and save a json post-build in the repo?
  2. That inlined script and styles inside faq.md look quite out of place for me. Would be nice to move somewhere else. The style is a global one, so it will mutate all H elements.
  3. We need to have a way to define categories. This is a pretty big trap, ppl will fuck this up, I guarantee.
  4. The order like that could be a pain to re-order. If one item needs to be inserted above the rest, you are fucked...
  5. How do you think those Jekyll Collections will help? I dont see Vuepress team responding on it at all :/

docs/.vuepress/components/FaqItem.vue Outdated Show resolved Hide resolved
docs/.vuepress/config.js Outdated Show resolved Hide resolved
docs/guide/learning/faq.md Outdated Show resolved Hide resolved
docs/guide/learning/faq.md Outdated Show resolved Hide resolved
docs/guide/learning/faq.md Outdated Show resolved Hide resolved
@@ -0,0 +1,41 @@
function slugify(input) {
Copy link
Owner

Choose a reason for hiding this comment

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

Cant we borrow this from Vuepress?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@sustained sustained Jan 5, 2020

Choose a reason for hiding this comment

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

I tried and it errors out within their code, I don't quite understand it.

VM20402 slugify.js:16 Uncaught TypeError: Cannot assign to read only property 'exports' of object '#<Object>'

I think it doesn't like that this part of their code is using Node-style exports.

Copy link
Owner

Choose a reason for hiding this comment

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

Most probably... we cant transpile their code I guess. ..

plugins/faqs/enhance.js Outdated Show resolved Hide resolved
plugins/faqs/enhance.js Outdated Show resolved Hide resolved
@sustained
Copy link
Contributor Author

sustained commented Jan 5, 2020

Sorry for big PR, it seems to be a habit of mine, I tried to keep it small. 😦


You said parsing the MD FAQ file was harder than this whole PR? Couldnt we have had an export task, that would just scan that file and save a json post-build in the repo?

Parse trees are complicated beasts, even for simple formats like Markdown but it's totally possible to process the Markdown parse tree into, say, a JSON file.

The primary issue with that approach is that the links are in the form of custom Vue components and a Markdown parser can't parse those into something we can work with.

Seeing your faq.md file but in parse tree form might make the problem become a bit more obvious.

That inlined script and styles inside faq.md look quite out of place for me. Would be nice to move somewhere else. The style is a global one, so it will mutate all H elements.

Good call, I will look at a better solution. ✔️

We need to have a way to define categories. This is a pretty big trap, ppl will fuck this up, I guarantee.

Can you elaborate on this point?

You think that categories should be defined somewhere? Instead of just being built based on whatever a user types in the YAML?

If we did that, then we could at least show a "the category you entered is invalid, please check categories.json" or something, if they make a typo. How does that sound?

The order like that could be a pain to re-order. If one item needs to be inserted above the rest, you are fucked...

Absolutely however I couldn't find any solution where it wasn't a pain, for example you could have an order field in the YAML FrontMatter but even then, if one thing needs inserting between the others then you're also fucked.

At first I actually didn't even bother ordering anything then I figured you might want me to maintain the current order / that things were in this order for a reason.

Open to suggestions.

How do you think those Jekyll Collections will help? I dont see Vuepress team responding on it at all :/

It's a powerful feature that can be applied to many things. Jekyll has it, Gatsby has a plugin for it, Gridsome has it, Saber is adding it in 1.0, it would probably be in their best interests to at least consider the idea but as I said, if they don't then maybe I'll make a plugin for fun/profit.

@sustained
Copy link
Contributor Author

I've fixed everything I can that doesn't require further feedback/discussion, I await your response. 👍

@sustained
Copy link
Contributor Author

sustained commented Jan 5, 2020

Here's a potential solution to the category-related issues.

image

So, category ordering would be based on the order in the plugin configuration (thus is easy to change). The plugin could also helpfully point out any invalid categories (or typos) at build time.

This doesn't solve the ordering of individual FAQ items, though. One potential solution for that is a common flag, which simply ensures that the "common" FAQ items come above the rest?


⬆️ Okay, that's all done. You even get these nice warning messages:

info FAQ Plugin Loading and processing FAQs.
warning FAQ Plugin 1-when-to-extract-components.md Unknown category componentss Did you make a typo or forget to define it docs/.vuepress/config.js?
warning FAQ Plugin 2-global-vs-local-components.md Unknown category componentss Did you make a typo or forget to define it docs/.vuepress/config.js?
warning FAQ Plugin 3-should-i-use-vue-files.md Unknown category componentss Did you make a typo or forget to define it docs/.vuepress/config.js?
warning FAQ Plugin 4-what-are-functional-components.md Unknown category componentss Did you make a typo or forget to define it docs/.vuepress/config.js?
info FAQ Plugin ...done!

But I will refrain from adding those commits until you've had a chance to go over the changes so far.

- Categories are now defined in the plugin configuration.
  - The ordering is the order they'll be displayed in
  - They have an optional description, if present it'll be displayed.
  - Any category in the frontmatter will be ignored/overwritten if present.
    - I guess I need to remove those in another refactoring sweep.
- If a contributor creates a Markdown file in a folder that doesn't correspond to a category in the config, or makes a typo when creating the folder, then they'll be warned at build time.
@sustained
Copy link
Contributor Author

sustained commented Jan 11, 2020

@dobromir-hristov

It's worth noting that the official Vue.js docs use an order: N property in the YAML frontmatter to determine the order of their documentation pages.

Happy to do the same for the FAQ items/questions if you think it's a good approach.

image

@sustained
Copy link
Contributor Author

I decided to just go ahead and do it, since we can always "revert commit" if necessary and I need a break from various other projects anyway. 😁

  • Categories are now specified in the plugin config (and their ordering is based on the ordering there).
  • FAQs are now ordered based on the order frontmatter property.

@sustained
Copy link
Contributor Author

I consider this done until review/more discussion can take place.

Original issue updated to reflect changes (i.e. only 1/5 potential issues remain).

Copy link
Owner

@dobromir-hristov dobromir-hristov left a comment

Choose a reason for hiding this comment

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

Gave it a review. Apart from not being able to use vue components inside the markdown, I dont see any other major issue.
That could be annoying if we want to re-use tags or other visuals, but I think its not such a big deal, we can have similar markdown based ones, right?

docs/.vuepress/config.js Outdated Show resolved Hide resolved
@sustained
Copy link
Contributor Author

Gave it a review. Apart from not being able to use vue components inside the markdown, I dont see any other major issue.
That could be annoying if we want to re-use tags or other visuals, but I think its not such a big deal, we can have similar markdown based ones, right?

Yeah, we could add a markdown-it plugin to the renderer that allows us to add badges in a more Markdown-esque way?

I've been looking into it but the VuePress code is quite complex (for me) and I'm struggling to piece together exactly how it achieves the Vue-components-within-markdown-files magic.

If/when I figure it out, I would like to add it. I just need to spend a lot more time stumbling through their codebase.

@sustained
Copy link
Contributor Author

sustained commented Jan 12, 2020

Okay, I just reviewed it too and found that typo.

I also extracted the duplication of the github repo to a const.

All done unless we want to fix the badge thing first. 👏

@sustained
Copy link
Contributor Author

I looked into creating a special markdown-it container for badges but I had issues.

For one the CSS relating to the Badge.vue component won't be included so it just renders as a plain-old span, for two I had issues creating an inline container as opposed to a block container.

I will keep looking into it but so far I feel stuck on this specific issue, no matter which approach I take.

@dobromir-hristov
Copy link
Owner

Hey sorry for the long wait. Should I merge this in? wtyt?

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