Prereqs For fixes - Githubissues

ryanhugh / searchneu

Search over Classes, Professors and Employees at NEU!

https://searchneu.com

GNU Affero General Public License v3.0

74 stars 18 forks source link

Prereqs For fixes #37

Closed edward-shen closed 6 years ago

edward-shen commented 6 years ago

Addresses most issues in #32.

edward-shen commented 6 years ago

While I'm not entirely done with #32 I got what we needed to make this prod ready. There are some questions though:

Why do we have a shouldUpdateComponent( ... ) function when it looks like it only checks for changed state? React automatically updates the component when there's a change to the state. Also still haven't gotten ordering completely down.

I might look into putting most of these helper functions into the base class panel instead, since it's just redundant code.

edward-shen commented 6 years ago

Ryan:

[x] Lets sort the prereqs for and opt prereqs for
[x] Lets make the show more button stand out from the classes more (different color?, does it look good if we make it the same color and font and style as the show more for sections?, not sure)
[x] Show more for required Prerequisite for (PSYC 2320: Statistics in Psychological Research)
[x] Lets add the show more to Prereqs too (ENGL 4710: Capstone Seminar) (not this PR - just an idea for the future :P)

ryanhugh commented 6 years ago

Looks good to me! Changed some of the styles and the Prereq section now looks like this.

One thing we could do is add a tooltip on top of 'Prerequisite for' and 'Optional Prerequisite for' to explain what they are. I'm down to merge for now and add that in the future.

ryanhugh commented 6 years ago

Feel free to merge into master!