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

Add image generation article #37

Merged
merged 10 commits into from Mar 22, 2024

Conversation

johnshaughnessy
Copy link
Collaborator

This PR adds an article, "Image Generation Guide".

It also modifies the build.sh script so that new articles are generated with the same name as existing articles (index-content.html).

It also enables support for gifs in the webpack config.

It removes the call for contributions about image-models (since this is meant to be that contribution).

It seems that all the generated files are named "index-content.html",
so update the build.sh script to reflect this.

Otherwise, running build.sh will generate a bunch of new index.html files.
Copy link

netlify bot commented Dec 29, 2023

Deploy Preview for mozilla-ai-guide ready!

Name Link
🔨 Latest commit 771fa8c
🔍 Latest deploy log https://app.netlify.com/sites/mozilla-ai-guide/deploys/6595f84e1b0b0b0008023db2
😎 Deploy Preview https://deploy-preview-37--mozilla-ai-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@skyfallsin
Copy link
Contributor

@johnshaughnessy can we patch up some of the styling - the rendered page looks odd. Dan can help

@johnshaughnessy
Copy link
Collaborator Author

johnshaughnessy commented Jan 3, 2024

@johnshaughnessy can we patch up some of the styling - the rendered page looks odd. Dan can help

Sure. I didn't added any css, so I'm guessing there are some styles added to the other pages that don't get picked up by the base template:

{% block page_css %}
<link rel="stylesheet" href="/css/ai.css"/>
{% endblock %}

Maybe these four inline stylesheets should be added to the base template?

./templates/content/choosing-ml-models/ai-guide-pick-a-model-test-a-model.html:7:<style type="text/css">
./templates/content/choosing-ml-models/ai-guide-pick-a-model-test-a-model.html:94:<style type="text/css">
./templates/content/choosing-ml-models/ai-guide-pick-a-model-test-a-model.html:6848:<style type="text/css">
./templates/content/choosing-ml-models/ai-guide-pick-a-model-test-a-model.html:7273:<style type="text/css">
./templates/content/choosing-ml-models/ai-guide-evaluate-ml-results.html:7:<style type="text/css">
./templates/content/choosing-ml-models/ai-guide-evaluate-ml-results.html:94:<style type="text/css">
./templates/content/choosing-ml-models/ai-guide-evaluate-ml-results.html:6848:<style type="text/css">
./templates/content/choosing-ml-models/ai-guide-evaluate-ml-results.html:7273:<style type="text/css">
./templates/content/comparing-open-llms/comparing-open-llms.html:7:<style type="text/css">
./templates/content/comparing-open-llms/comparing-open-llms.html:94:<style type="text/css">
./templates/content/comparing-open-llms/comparing-open-llms.html:6848:<style type="text/css">
./templates/content/comparing-open-llms/comparing-open-llms.html:7273:<style type="text/css">

@johnshaughnessy
Copy link
Collaborator Author

johnshaughnessy commented Jan 3, 2024

As I've been poking around, I think I figured out that the 4 additional inline stylesheets come from the ipynb -> html conversion process (nbconvert)

Example: 93e5c43#diff-ab61d65c3666803d258e942605930e314dd946ec56313c526f9eb276b6f5ac02R33

There are no ipynb's in my article, so those styles didn't get added. I suspect I can fix my article in any of these three ways:

  • adding an ipynb and building with tools/build_ai_guide.sh (rather than templates/content/build.sh, which is what I've been using) ,
  • manually include the 4 inline styles to the generated html or
  • add the 4 inline styles to the base template (so that every page gets it, whether they have ipynbs or not)

I'm not sure which way is simplest. I'll think about it as I'm trying them.

Otherwise, headerIds are disabled by default:

    markedjs/marked#2890

Supposedly, there is a way to to use a configuration file:

    marked -c .marked.json -o "${NEW_PATH%.md}-content.html" "${file}"

with .marked.json:

{
  "headerIds": true
}

But I could not get this working with the latest version of marked (11).
@johnshaughnessy
Copy link
Collaborator Author

johnshaughnessy commented Jan 4, 2024

Ok, I made two discoveries.

  1. I was running into a problem with ./tools/build_ai_guide.sh where none of the headers were getting auto-generated ids. This is because recent versions of marked disable auto-generated ids for headers by default. I couldn't get marked to respect an alternative configuration file, so instead I updated ./tools/build_ai_guide.sh so that it installs marked@^6.0.0. More details are in the commit comment.

  2. The four in-line stylesheets that I mentioned above were red-herrings. Adding these to my guide didn't do anything to change the styles. I noticed that my guide uses headings h1, h2, h3, and h4, whereas the other articles seem to favor h2 and h4. It turns out there were no rules setup for h1 and h3. This accounts for the visual difference. To fix this, I added basic style rules for h1 and h3.

This is ready for re-review @skyfallsin . If you notice any other oddities with the styles, please let me know what they are. It took me a while to notice what you might be referring to with your previous comment.

@johnshaughnessy johnshaughnessy merged commit 0a16026 into mozilla:main Mar 22, 2024
4 checks passed
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.

None yet

2 participants