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

Foundations/JS basics part 2: Updated list styles #27157

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

damon314159
Copy link
Member

Because

List styles in lesson file don't match the new style guide

This PR

Makes the required changes to bring the style in line

Issue

Contributes to #27119

Additional Information

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project Contributing Guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

@github-actions github-actions bot added the Content: Foundations Involves the Foundations content label Jan 18, 2024
@damon314159
Copy link
Member Author

damon314159 commented Jan 19, 2024

@thatblindgeye slight roadbump on this one. The linter isnt happy about the heading used to give the Replit warning note it's title
When you check the linter configs:

"MD043": {
      "headings": [
        "### Introduction",
        "### Lesson overview",
        "*",
        "### Assignment",
        "### Knowledge check",
        "### Additional resources"
      ],
      "match_case": true
    }

It allows * wildcards only between lesson and assignment, but since this note falls between assignment and knowledge, there's no way to make it pass.

Either the note needs moving way up the page (where it will be nowhere near replit assignment), or the linter config needs an asterisk between assignment and knowledge check.

Happy to PR the linter configs with a green light. Thoughts?

@thatblindgeye
Copy link
Contributor

Yep, some of our lessons will trigger this error unfortunately. It may be worth us trying to customize our own rule for heading structures rather than using the default rule (which only allows this sort of limitation, either an exact structure or wildcard of anything).

I'm sort of leaning towards removing that heading as I'm not sure it's totally needed... What do you think? Would be interested from the user's perspective in terms of what is helpful when going through these lessons.

@damon314159
Copy link
Member Author

If you take out the h4 heading you don't get the warning triangle, so people might gloss over it.
Even if they don't it feels like a bandage over a larger issue, that sometimes you might need notes in places the template isnt expecting

@thatblindgeye
Copy link
Contributor

The styling is set up so that any note boxes render with an icon whether there is a heading inside it or not, so that wouldn't be an issue. But agreed that the Assignment section at least may have level 4 headings. That's something that would be a follow up issue, though. For now I'll leave it up to you whether to remove that heading or not, as I don't feel super strong about it. I've opened #27162 for now as a follow up

@damon314159
Copy link
Member Author

As the config file was changed, this test would no longer fail if rerun - but I don't have the permissions to rerun it @thatblindgeye

@JoshDevHub
Copy link
Contributor

@damon314159 reran and it still fails

Probably have to merge main into this branch to get the necessary config update

@damon314159
Copy link
Member Author

That was a hard fought battle 😆 - bot is finally happy

@thatblindgeye thatblindgeye merged commit a620682 into TheOdinProject:main Jan 23, 2024
3 checks passed
@damon314159 damon314159 deleted the patch-3 branch January 23, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: Foundations Involves the Foundations content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants