Scroll Comment Fixed #112

2020-11-18T09:22:02+01:00

Yash-R commented

2020-11-18 09:22:02 +01:00

(Migrated from github.com)

resolved #83

Irev-Dev (Migrated from github.com) requested changes 2020-11-18 18:52:38 +01:00

Irev-Dev (Migrated from github.com) left a comment

Looks good @Yash-R, I've just asked for a couple of tweaks.

Also I noticed there are changes to the submodule and to yarn lock that don't seem related. Where these changes intended? maybe try

git submodule update --recursive --remote
git pull
git merge master
git push

on your branch, might fix it.

Looks good @Yash-R, I've just asked for a couple of tweaks. Also I noticed there are changes to the submodule and to yarn lock that don't seem related. Where these changes intended? maybe try ``` git submodule update --recursive --remote git pull git merge master git push ``` on your branch, might fix it.

web/src/components/PartProfile/PartProfile.js

Irev-Dev (Migrated from github.com) commented

2020-11-18 18:43:54 +01:00

Could we change this to Comments {userPart.Part.Comment.length}. Otherwise it's not clear what the button is for.

Could we change this to `Comments {userPart.Part.Comment.length}`. Otherwise it's not clear what the button is for.

web/src/components/PartProfile/PartProfile.js

Irev-Dev (Migrated from github.com) commented

2020-11-18 18:46:43 +01:00

Could we make this id less generic? id="comment-section"

As much as possible we want our code to express intent, that way it will be easier to figure out what's going on later, or when someone else is reading it and document.getElementById('section').scrollIntoView() doesn't tell us much besides that it's scrolling somewhere. though document.getElementById('comment-section').scrollIntoView() tells us the intention nicely.

Could we make this id less generic? `id="comment-section"` As much as possible we want our code to express intent, that way it will be easier to figure out what's going on later, or when someone else is reading it and `document.getElementById('section').scrollIntoView()` doesn't tell us much besides that it's scrolling somewhere. though `document.getElementById('comment-section').scrollIntoView()` tells us the intention nicely.