Exercise 8 #10
Exercise 8 #10
Conversation
…works/no-js-pls into exercise-7
roschaefer
left a comment
roschaefer
left a comment
There was a problem hiding this comment.
Ok, folks, if I'd be super strict I had to deduce a point because your items do not switch to two column layout on mobile devices. I won't this time, because you extended your previous exercises and never had anything to put into the left colmn. I like that you implemented many 🚀 this time. Well done! You received 5/5 ⭐
There are some bugs with your frontend, see below.
⭐ For a header and it's responsiveness (direct links vs. hamburger menu)
⭐ For a content section and it's content items
⭐ For a responsive content section (3-column vs 1-column layout)
⭐ For responsive content items (1-column vs. 2-column layout) (You have extended exercises 1-7 and don't have an image on the items.)
⭐ For a footer
🚀 For a smooth transition when the sidebar is revealed or hidden. Use VueJS transitions.
🚀 For a sidebar containing all the navigation links. The sidebar is revealed when the hamburger menu button is clicked and can be closed.
🚀 For extending exercise 1-7 (instead of creating a new project) and adapting the content section (to contain title and votes)
🚀 For using semantic html.
| <template> | ||
| <form class="login-form" @submit.prevent="submitLogin"> | ||
| <fieldset> | ||
| <legend> |
| /* | ||
| @Component | ||
| export default class LoginForm extends Vue { | ||
| @Prop() public token: string = this.$store.state.auth.token; | ||
| public email: string = ''; | ||
| public password: string = ''; | ||
| @Action('auth') | ||
| async submitLogin(): Promise<void> { | ||
| const { login } = await this.$a | ||
| const res = await this.$apollo.mutate({mutation: gql`mutation ($email: String!, $password: String!){ | ||
| login(email: $email, password: $password) | ||
| }`, variables: {email: this.email, password: this.password}}).then((data: any) => data.login as string); | ||
| await this.$app.$apolloHelpers.onLogin(res); | ||
| this.$store.commit('login', { email: this.email, password: this.password }); | ||
| } | ||
| }; */ | ||
| </script> | ||
|
|
||
| <style scoped> | ||
| </style> |
There was a problem hiding this comment.
| /* | |
| @Component | |
| export default class LoginForm extends Vue { | |
| @Prop() public token: string = this.$store.state.auth.token; | |
| public email: string = ''; | |
| public password: string = ''; | |
| @Action('auth') | |
| async submitLogin(): Promise<void> { | |
| const { login } = await this.$a | |
| const res = await this.$apollo.mutate({mutation: gql`mutation ($email: String!, $password: String!){ | |
| login(email: $email, password: $password) | |
| }`, variables: {email: this.email, password: this.password}}).then((data: any) => data.login as string); | |
| await this.$app.$apolloHelpers.onLogin(res); | |
| this.$store.commit('login', { email: this.email, password: this.password }); | |
| } | |
| }; */ | |
| </script> | |
| <style scoped> | |
| </style> |
git?
| }, | ||
| mounted() { | ||
| this.setupMenu(); |
There was a problem hiding this comment.
There is a better way to do this which also supports server-side-rendering. See step 5: https://github.com/Systems-Development-and-Frameworks/homework/tree/main/exercises/7#instructions
Setting the token in mounted will make the unauthenticated page is visible for a second. Plus, you get this nasty "boiling hydration" runtime error.
| logout() { | ||
| this.token = null; | ||
| this.$apolloHelpers.onLogout(); | ||
| window.location.reload(false); |
There was a problem hiding this comment.
Try to avoid window and document because of SSR. This might not be a problem here, but it's good practice to simply avoid it.
There was a problem hiding this comment.
Also, I don't understand why you reload the page. The authentication state should be reactive and components should react to it.
There was a problem hiding this comment.
Forcing a page reload contradicts the whole purpose of using a SPA.
| <button @click="updateNews(-1)">Downvote</button> | ||
| <button @click="deleteNews">Remove</button> | ||
| <p> | ||
| Lorem ipsum dolor sit amet, consectetur adipiscing elit. Fusce consequat pulvinar convallis. Vivamus iaculis, |
There was a problem hiding this comment.
Because you seem to be such a big fan of deeply nested npm dependencies: You know lorem-ipsum? https://github.com/knicklabs/lorem-ipsum.js#readme
Even more: https://github.com/templeman/awesome-ipsum
| }, | ||
| methods: { | ||
| updateNews(value) { |
There was a problem hiding this comment.
You have a bug here. Clicking downvote and upvote has the same effect.
| <button class="text-black bg-gray-200 hover:bg-gray-700 hover:text-white px-3 py-2 rounded-md text-sm font-medium" v-if="token" @click="updateNews(1)">Upvote</button> | ||
| <button class="text-black bg-gray-200 hover:bg-gray-700 hover:text-white px-3 py-2 rounded-md text-sm font-medium" v-if="token" @click="updateNews(-1)">Downvote</button> | ||
| <button class="text-black bg-gray-200 hover:bg-gray-700 hover:text-white px-3 py-2 rounded-md text-sm font-medium" v-if="authorId === news.author.id">Edit</button> | ||
| <button class="text-black bg-red-300 hover:bg-red-600 hover:text-white px-3 py-2 rounded-md text-sm font-medium" v-if="authorId === news.author.id" @click="deleteNews">Delete</button> |
There was a problem hiding this comment.
That's where I get the following error:
client.js?06a0:97 TypeError: Cannot read property 'id' of undefined
at Proxy.render (NewsItem.vue?a1bb:57)
at VueComponent.Vue._render (vue.runtime.esm.js?2b0e:3548)
at VueComponent.updateComponent (vue.runtime.esm.js?2b0e:4055)
at Watcher.get (vue.runtime.esm.js?2b0e:4479)
at new Watcher (vue.runtime.esm.js?2b0e:4468)
at mountComponent (vue.runtime.esm.js?2b0e:4073)
at VueComponent.Vue.$mount (vue.runtime.esm.js?2b0e:8415)
at init (vue.runtime.esm.js?2b0e:3118)
at createComponent (vue.runtime.esm.js?2b0e:5978)
at createElm (vue.runtime.esm.js?2b0e:5925)
|
@Systems-Development-and-Frameworks/pls-no-js your team has received 65/70 ⭐ for all homeworks, great 👍 |
Added styling for the main page with Tailwind.