Left to right prerequisites trees

[zehata]

Context

Fixes #3202 Prerequisite tree right-to-left progression is confusing

Implementation

I referenced the previous 2 PRs #3263 and #3291 to see what was good and why they were not merged.

Having noted https://github.com/nusmodifications/nusmods/pull/3263#discussion_r862281206, the implementation was changed to modify the DOM instead of using flex-direction: row-reverse; I fully copied @sciffany's copywriting since they are indeed easier to understand, so all credits to her.

Having noted the test cases used by @alvynben in PR #3291, I elected to use those cases (though with different modules).

I also noted that the last correspondence for both PRs were in 2021, so I believe opening a new PR should be appropriate.

Options in user settings?

I noted that Issue #3202 specifically asked for left-to-right prerequisites trees, so this PR only targets that specification. However, I would favour closing #3202 and creating a new issue for the same but with an option in the settings that allow users to choose their preference. Having gotten used to the old prerequisites trees, users like myself might choose to stick to that. However, I also note that a left-to-right prerequisites tree is definitely more intuitive so it should not be counted out as well.

UI tests

For a single mod, with no prerequisite or dependant: For mods with only dependants: For mods with one layer of prerequisites and dependants, also for mods with requireNum modules: For mods with two layers of prerequisites: For mods with uneven layers of prerequisites: Highly complex prerequisites, prerequisites with uneven number of letters:

Other Information

Any questions you may have

Is there a tele group specific for development?

Letting us know that you're new to this (so we know how much to help out)

Been almost 3 years since I touched any web dev. Sorry in advance if I had missed out anything.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
nusmods-export	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 23, 2024 1:50pm

[vercel[bot]]

@zehata is attempting to deploy a commit to the modsbot's projects Team on Vercel.

A member of the Team first needs to authorize it.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 54.66%. Comparing base (2d4743d) to head (c70c538). Report is 42 commits behind head on master.

Files with missing lines	Patch %	Lines
website/src/views/modules/ModuleTree.tsx	87.50%	1 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3840      +/-   ##
==========================================
+ Coverage   53.87%   54.66%   +0.78%     
==========================================
  Files         274      274              
  Lines        6032     6072      +40     
  Branches     1449     1455       +6     
==========================================
+ Hits         3250     3319      +69     
+ Misses       2782     2753      -29     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[zehata]

Fixes #3202

[zehata]

I wanted to refactor the style class names to use left or right instead of preReq- so that it is easier to understand, but that is beyond the scope of #3202. Will open a separate PR if that is required.

[zehata]

Re: https://github.com/nusmodifications/nusmods/pull/3840/files#diff-c16227ad8088f0a3a8edf1ae53344bb0189198cb956157ec0a9083cf3e7069f9L106-L112 The code originally had prereqNode class on conditional nodes. As far as I understand, modules don't conditionally unlock other modules, i.e. there are no situations like so:

B —┐
C —|— needs (isConditional == true) — A 
D —┘

so isConditional is never true and this style declaration is unused,

https://github.com/nusmodifications/nusmods/pull/3840/files#diff-c16227ad8088f0a3a8edf1ae53344bb0189198cb956157ec0a9083cf3e7069f9R121 omits {[styles.prereqNode]: isPreReq} since there are never a situation whereby:

                    ┌— B
A — unlocks one of —|— C
                    └— D

[zehata]

Update: I have a working copy of a version with a toggle in settings, but I am still refactoring. See preview at temporary branch https://github.com/zehata/nusmods/tree/zehata/website-module-page/3202/toggleable-left-to-right-prerequisites-trees-wip

[zehata]

Reverted to draft because of

Update: Looks like it was already like this, just not visible when it was on the right. Fixed regardless.

[jloh02]

I think to allow this feature to have a toggle switch setting in future, it would be good to also have a boolean parameter to switch back to right-to-left.

[zehata]

Rebased onto 0f33a55

[zehata]

Toggleable implementation extending this PR is at #3842. Please note that the branch in that PR diverges from this branch and cannot both be merged.

[zwliew]

Since we've already merged #3842, this shouldn't be needed anymore.