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

Dynamic User Interface Interactions: Removing the requirement for smooth transition #27247

Closed
3 tasks done
Sokolan opened this issue Jan 31, 2024 · 11 comments · Fixed by #27846
Closed
3 tasks done

Dynamic User Interface Interactions: Removing the requirement for smooth transition #27247

Sokolan opened this issue Jan 31, 2024 · 11 comments · Fixed by #27846
Assignees

Comments

@Sokolan
Copy link
Contributor

Sokolan commented Jan 31, 2024

Checks

Describe your suggestion

Currently in the "Dynamic User Interface Interactions" the assignment of `Image slider" instruction .3 is as follows:

Once you have the slider positioned properly, build functions for “next” and “previous” which will advance to the next or previous slide accordingly. Make the transitions smooth using effects.

The issue is that animations haven't been introduced yet, they appear only in the next course "Advanced html and css".

My suggestion is to remove this requirement, as the main goal of this practice (as far as I understand it) is to practice JavaScript techniques to build the slider, and less emphasis on the css part.

Path

Node / JS

Lesson Url

https://www.theodinproject.com/lessons/node-path-javascript-dynamic-user-interface-interactions

(Optional) Discord Name

Sokolan

(Optional) Additional Comments

No response

@CouchofTomato
Copy link
Member

@TheOdinProject/javascript can someone review please.

@thatblindgeye
Copy link
Contributor

thatblindgeye commented Feb 3, 2024

I'd agree we should remove that requirement. For context, I'm pretty sure this lesson was written before the HTML/CSS overhaul, which would explain why that requirement is there. I don't think we should be asking users to put a pause on this lesson to go learn about something that's required for this lesson, when we'll be covering that sort of topic not long after.

We may need to do more than just remove that requirement, e.g. remove the blockquote linking to the Youtube video, maybe rewriting some verbiage, etc.

@Sokolan
Copy link
Contributor Author

Sokolan commented Feb 4, 2024

So beside removing the requirement in the assignment, you're also suggesting to remove the quote in the introduction.
Maybe the "Mobile menus" part is also a little out of the scope here? as responsive design is being introduced in "Advance HTML and CSS". This change will lead to the questions in the "Knowledge check" to be irrelevant tho (so removing those too?).

@thatblindgeye
Copy link
Contributor

There's a comment in a discussion kinda related to this: #26390 (reply in thread)

I'd agree with removing the Mobile menu portion of this lesson, though, in addition to what was mentioned above. Plus any other necessary changes (such as knowledge checks) that would be affected.

Copy link

This issue is stale because it has had no activity for the last 30 days.

@github-actions github-actions bot added the Status: Stale This issue/PR has been inactive for over 30 days and will be closed if inactivity continues label Mar 14, 2024
@thatblindgeye thatblindgeye removed the Status: Stale This issue/PR has been inactive for over 30 days and will be closed if inactivity continues label Mar 14, 2024
@MaoShizhong
Copy link
Contributor

MaoShizhong commented Mar 28, 2024

https://discord.com/channels/505093832157691914/505093832157691916/1222644707587526726

A recent case where another learner was confused by the "smooth transitions" requirement despite not coming across transitions yet.

I think at the very least, while other ideas are being discussed in that linked discussion, we should do a small PR that just removes that one "smooth transitions" sentence. Perhaps even replace it with "You do not need to animate the slides changing."

IMO this will reduce confusion ASAP, then any further changes to this specific lesson could be proposed and discussed in a separate issue as they're not directly related to this specific one.
Larger changes may well come in the future as a result of that linked discussion.

Thoughts @thatblindgeye? If sensible, we can assign Sokolan to have that sentence removed/replaced to resolve this particular issue.

@thatblindgeye
Copy link
Contributor

I think at the least as part of this issue we should:

  • remove the mention of smooth transitions in the "image slider" section
  • remove the entire mobile menu section (but keep the dropdown menu content)
  • remove the blockquote in the Introduction that links to the video on animations

I'll post in the linked discussion again to try and get the ball rolling there for any further possible updates to this lesson.

@MaoShizhong
Copy link
Contributor

I think those three suggestions make perfect sense, and would be happy to see them go.

@thatblindgeye
Copy link
Contributor

@Sokolan would you be interested still in making the updates discussed above?

@Sokolan
Copy link
Contributor Author

Sokolan commented Apr 9, 2024

Sorry for the late response.
I'll be glad to implement those changes, will make a PR in the next few days.

@MaoShizhong
Copy link
Contributor

MaoShizhong commented Apr 9, 2024

@Sokolan While making those changes, would you be able to also apply the following link-based changes as per the following spreadsheet?

https://docs.google.com/spreadsheets/d/1E1OJqUfBODvjdFMNgFMY_GvOLrOzV4ujL9o7Nxzg2zk/edit?usp=sharing

You can find the instructions in the JavaScript tab, under the Dynamic User Interface Interactions rows. Just a small link text change.

Let me know if you have any issues with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment