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

Update intro-to-css.md #28754

Merged
merged 7 commits into from
Sep 9, 2024
Merged

Update intro-to-css.md #28754

merged 7 commits into from
Sep 9, 2024

Conversation

abhilash15500
Copy link
Contributor

@abhilash15500 abhilash15500 commented Sep 7, 2024

A warning would be very helpful as i see people in discord everyday asking doubts about animation exercise. They solve 1to5 exercises and then in flow they start animation even when its instructed. I think a warning should make them aware to not touch anything else except foundations 1 to 5 exercises.

Because

This PR

Issue

Closes #XXXXX

Additional Information

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum 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

A warning would be as i see people in discord everyday asking doubts about animation exercise. They solve 1to5 exercises and then in flow they start animation even when its instructed.
I think a warning should make them aware to not touch anything else except foundations 1 to 5 exercises.
@github-actions github-actions bot added the Content: Foundations Involves the Foundations content label Sep 7, 2024
@XAJX179
Copy link
Contributor

XAJX179 commented Sep 7, 2024

hello xplozion! just a heads up remove the blank lines at line numbers - 364 , 373 to avoid markdown lint errors and following style guide.

Now on topic -
i think we should add some writing about reading instructions more carefully in future too, if we add this warning.

Removed gaps on line 363 and 374
@MaoShizhong
Copy link
Contributor

MaoShizhong commented Sep 7, 2024

Thanks for the PR. Honestly, I'm not too sure this will change much. The assignment very clearly already states to go to the foundations folder and complete the listed exercises. People have generally only done differently when they get carried away and/or just straight up forget the instructions after reading the README but don't go back to check them (so they assume).

In those cases, given the explicit instructions they read to get to that repo had been forgotten, I'm not convinced a note box repeating those instructions beforehand will change this. i.e. there's a problem (people not reading instructions) but I don't think that's a problem we can reasonably solve given our instructions are already so explicit. I'll see what other maintainers have to say though, if any have differing opinions.

I will point out though that your note box doesn't follow the correct syntax. The heading should be a level 4 heading placed inside the div, and should also ideally be more descriptive than a generic "warning".

@Eduardo06sp
Copy link
Member

@MaoShizhong Is it okay to openly discuss some potential solution to this problem here?

I share the same concern as the author and have seen multiple users (in the Discord) accidentally go to the more advanced exercises.

@abhilash15500
Copy link
Contributor Author

One solution i can think is directly redirecting them to foundations folder instead of css exercises? That would be better I guess

@MaoShizhong
Copy link
Contributor

MaoShizhong commented Sep 8, 2024

@Eduardo06sp Please do fire away. PRs are public and open to discussion as appropriate .If there is a solution, I'm not sure a note box would be it but perhaps there's another solution that's suitable.

And @abhilash15500 we need people to read the README first (and even when we say it clear and link to it, some people still don't read it and we can't really help prevent those cases).

Perhaps we could split the assignment into 2 steps. The first one instructs to read the repo README, linking as it currently does. The second step then links to the Foundations folder and instructs to do exercises 1-5 as it currently does. I'd love to hear what Eduardo wants to say as well

@abhilash15500
Copy link
Contributor Author

Splitting assignment would work nicely.
Also mb for not raising issue first i was unaware of the workflow for this.
Just shared what i felt like is an issue.

I would be happy with whatever steps TOP management and core members take for this 😄

@MaoShizhong
Copy link
Contributor

Yes ideally an issue is created first so the proposal can be made (examples if appropriate) then it can be discussed, just in case things either don't get approved or perhaps a different solution is more appropriate through discussion, especially if you haven't had a lot of experience with contributing here. Of course if it's a super obvious fix like a typo or broken link, then by all means go straight to a PR if there isn't already an open issue/PR for it.

@abhilash15500
Copy link
Contributor Author

I will remember it next time.

Im happy to follow whatever steps needed if any from my side / or any decision which top members take which they think its for the betterment of TOP! :)

@Eduardo06sp
Copy link
Member

@MaoShizhong I think that small change would help a lot.

I also believe that structuring the css-exercises repo like the curriculum is structured would serve as another "fail-safe".

I.e.
Instead of the folders being laid out like they currently are

animation  flex  foundations  grid  margin-and-padding

we can structure them just like they appear in the curriculum

foundations  intermediate-html-and-css  advanced-html-and-css

and the nesting can go deeper if deemed necessary,
but the drawback is that any potential, future updates to the curriculum's structure will require an update in the css-exercises repo as well

just an idea :}

@MaoShizhong
Copy link
Contributor

Thanks @Eduardo06sp. Could you open an issue about that in the css-exercises repo please? I think that's something better proposed and discussed there (and I'd like input from other team members), and if it gets approved, there will also be a sibling PR here to amend any lessons to reference the right folders and files etc.

Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

Regarding this PR, @abhilash15500, could you remove the note box changes you originally made then do the following?

  • Split the assignment step into 2 steps:
    1. Link to the CSS exercises repo and instruct to read the README
    2. Link to the foundations directory and carry on with the rest of the instructions.

… the exercises to solve.

Update to assignment section in  intro-to-css.md for clarity regarding the exercises to solve. Made changes as suggested.
Made changes as suggested in PR.
@abhilash15500
Copy link
Contributor Author

@MaoShizhong I commited the changes as suggested. Can you please take a look at it? Do tell me if any changes are needed from my side.

Remove extra spaces intro-to-css.md
…ss.md

Fix ordered list numbering to comply with linting rules in intro-to-css.md
More accesible link text label.

Co-authored-by: MaoShizhong <[email protected]>
Copy link
Contributor

@MaoShizhong MaoShizhong left a comment

Choose a reason for hiding this comment

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

🙏

@MaoShizhong MaoShizhong merged commit 9f333a6 into TheOdinProject:main Sep 9, 2024
2 checks passed
@abhilash15500 abhilash15500 deleted the patch-2 branch September 9, 2024 12:50
@abhilash15500
Copy link
Contributor Author

Appreciated! ☺️

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.

4 participants