Closed cerobe closed 10 months ago
Updated the PR according to the feedback. Waiting for the alignment on the last review, should we put the VM in the example and put a comment about this loosely breaking MVVM.
Apart from the things I mentioned and my previous concerns about the VM, it looks good. Feel free to mark all the previous reviews except the VM one resolved.
Thanks for your detailed and high quality review @thevortexcloud. I've updated the PR with your latest feedback and closed them. Only the VM is still open and waiting for @timunie input.
You are welcome! I am fan of good documentation. Which is why I try to make sure documentation is reasonably detailed and has some sort of logic behind what it's saying.
EDIT: For the record though, someone like Tim will need to approve this as I am not in charge around here.
I've update this PR to fix https://github.com/AvaloniaUI/avalonia-docs/issues/326.
FYI, the NPM build is now failing seemingly as a result of some change here:
[success] [webpackbar] Client: Compiled with some errors in 35.62s Error: Client bundle compiled with errors therefore further build is impossible. Error: Docs markdown link couldn't be resolved: (../reference/controls/itemscontrol.md) in "/home/runner/work/avalonia-docs/avalonia-docs/docs/concepts/custom-itemspanel.md" for version current at Array.forEach (
)
Module not found: Error: Can't resolve '/img/gitbook-import/assets/items.gif' in '/home/runner/work/avalonia-docs/avalonia-docs/i18n/ru/docusaurus-plugin-content-docs/current/reference/controls' Error: Process completed with exit code 1.
Yes there seems to be in the merge as I changed this. As you can see in the changed files.
Let me try to get the right value and push a fix.
Found it, it was files for translation... rooky mistake, when moving an img, don;t assume anything and make a research on the whole code base for rogue imports :D
Tht should be fine...I guess, I could not check as I changing the language when running locally is not working for me (am I doing something wrong by using the language listbox?)
Tht should be fine...I guess, I could not check as I changing the language when running locally is not working for me (am I doing something wrong by using the language listbox?)
Not sure, I have never run the docs site locally before. In any case, it's still failing with the same error.
[success] [webpackbar] Client: Compiled with some errors in 35.17s
Error: Client bundle compiled with errors therefore further build is impossible.
Error: Docs markdown link couldn't be resolved: (../reference/controls/itemscontrol.md) in "/home/runner/work/avalonia-docs/avalonia-docs/docs/concepts/custom-itemspanel.md" for version current
at Array.forEach (<anonymous>)
hum, I'm at a loss here. The links seems correct and it's working locally (I know it's programmer meme). Any Idea what is wrong?
@cerobe probably case sensitive. Other than that I need to take a look next week or ask the team.
Have you been able to look at the issue @timunie ?
@cerobe can you try my suggestion please? I am not 100 % sure but it is what I found in the docusaurs docs
[
ItemsControl](../reference/controls/itemscontrol.md)
can you try to add a leading ./?[ItemsControl](./../reference/controls/itemscontrol.md)
Not sure it solves our issue but the links are still working for me and probably it is what the engine requires: https://docusaurus.io/docs/next/markdown-features/links
@timunie done, it's indeed still working in local. let's see if fixes the bug.
Build is failing with Error: Docs markdown link couldn't be resolved: (./../reference/controls/itemscontrol.md) in "/home/runner/work/avalonia-docs/avalonia-docs/docs/concepts/custom-itemspanel.md" for version current
@cerobe looks like omitting the .md extension helps. https://github.com/AvaloniaUI/avalonia-docs/pull/341/commits/1636b6b41e909fd6f03ab96dd95edfc94851cbdc
I HAVE NO IDEA WHY AND IF LINKS WORKS AFTERWARDS. But it is the only option I found so far. 🤷
@timunie , weird one, should I try to remove the .md extension on all links in this page then?
should I try to remove the .md extension on all links in this page then?
Yes, please let us give that a try.
Good news, looks like the build worked!
That was completely unexpected, but nice that you found that. WE should be able to merge this PR soon...Finally.
That was an interesting drive 👀
Thank you for your contribution and patience 🙏
It's live and it looks like the links work.
fixes #326