Fix frontend compile errors + schedule padding #19
Conversation
…rd padding is removed for all views however
| @@ -306,7 +307,8 @@ export default { | |||
|
|
|||
| api | |||
| .updateScheduleName(this.schedule[index].id, cleanedName) | |||
| .then(() => this.schedule[index].name = cleanedName) | |||
| .then(() => this.scheduleLocal[index].name = cleanedName) // steven: put schedule as a local variable in data (line 129) to avoid mutation error, not sure if right move | |||
There was a problem hiding this comment.
What was the error you were getting?
There was a problem hiding this comment.
This particular one is the 'no-mutating-props' error (https://eslint.vuejs.org/rules/no-mutating-props.html) and this occurs when we try to set a value to a prop in a component such as 'this.prop = value'. So I tried storing the prop as a local data variable (scheduleLocal), which got rid of the error.
I fixed the 'multi-word-component-names' error by just turning it off, and I fixed the "no-prototype-builtins" error by calling the method from Object.prototype as stated in https://eslint.org/docs/latest/rules/no-prototype-builtins
There was a problem hiding this comment.
I pulled your PR to a Windows machine and M1 Mac, using fresh installs of Node 18 and Java 8. Everything built and ran successfully.
@WinbertZhang If you agree, let's get this merged ASAP to unblock everyone
@stychoi Could you comment on the PR thread with more details about how you fixed this? This issue was plaguing a lot of the team, and you seemed to have cracked it. Maybe you can give a summary in Wednesday's meeting?
There was a problem hiding this comment.
I spoke too soon, it looks like one of the changes in the package-json is breaking the unit tests: https://github.com/data-science-ucsb/gauchocourses/actions/runs/3719544630/jobs/6308470011
I might have found a solution here: #21
|
Try updating this branch from main to fix the CI errors Issue link: crunch-time/crunchtime#6
|
|
Looks like the pr was already merged with pr #21 but I'll take another look and make another pr for the list view padding |
Resolves issue #16 but would like another pair of eyes to see if my changes are okay.
Also worked on the schedule padding issues from crunchtime crunch-time/crunchtime#6
Still not an expert at Vue.js and the styling so would appreciate some code review to make sure anything else didn't break. Thanks!