Pull request for homework 7 #10
Conversation
There was a problem hiding this comment.
I haven't tested your code, but it loosk very decent!
For some reason, your PR does not contain relevant code, e.g. how you use store.readQuery and store.writeQuery, but I could expand some of the folds and found it there. So you learned how to use it, great!
The $accessor thing is required because of typescript, right?
This is just a preliminary feedback, no ⭐ given yet.
| @@ -21,7 +23,7 @@ const newsItemsMock = [ | |||
| author: { | |||
| id: '1', | |||
| name: 'Christoph Stach', | |||
| email: 's0555912@htw-berlin.de', | |||
| email: 's0555912@htw-berlisn.de', | |||
There was a problem hiding this comment.
Anonymizing your personal data? 😉
| it('exists', () => { | ||
| const wrapper = mount(NewsList, { | ||
| localVue, | ||
| it('should exist', () => { |
There was a problem hiding this comment.
Don't use should. Be affirmative and say it exists, period. Or maybe a more specific test case description.
| }) | ||
|
|
||
| if (process.client) { | ||
| window.localStorage.setItem('token', result.data.login) |
There was a problem hiding this comment.
Things like window and document cause problems in SSR 👍
webapp/nuxt.config.js
Outdated
| apollo: { | ||
| clientConfigs: { | ||
| default: { | ||
| httpEndpoint: 'https://news-list-backend.herokuapp.com', | ||
| authenticationType: 'Bearer', |
There was a problem hiding this comment.
| authenticationType: 'Bearer', |
Is already the default: https://github.com/nuxt-community/apollo-module#advanced-configuration
| return this.$accessor.auth.isLoggedIn | ||
| }, | ||
| }, | ||
| created() { |
There was a problem hiding this comment.
Better use a middleware here: https://nuxtjs.org/docs/2.x/directory-structure/middleware/
There was a problem hiding this comment.
Good mention which also helps me because I also used created()
| export default Vue.extend({ | ||
| computed: { | ||
| isLoggedIn() { | ||
| return this.$accessor.auth.isLoggedIn |
There was a problem hiding this comment.
For what do you need the computed property here? 🤔
There was a problem hiding this comment.
Hi Team!
I like the design of your frontend! It looks nice and clean!
You got many changes in your "Files Changes" tab (~30% more than the others) , that's probably because of typescript again.
Thanks for the screenshot of the Lighthouse reporting!
❌ You're missing out on one ⭐, because I could not find any test for the visibility of the login/logout button. If you can proof me wrong, I am happy to correct that.
⭐ For instructions in the README.md on how to build your webapp for production.
⭐ For no changes in "Files Changed" tab of the refactoring from vue-cli to create-nuxt-app. (See #1 in instructions)
⭐ ⭐ For the API connection between your front- and backend.
⭐ For your previous frontend tests still passing. Requests to the backend are mocked.
⭐ ⭐ For a login feature in your webapp including a Vue component and its software tests.
⭐ ❌ For a menu component which shows a login or logout button and its software tests.
⭐ For an upvote button that behaves according to the authentication state of your user
⭐ For a delete and edit button that is only visible to the author of the post.
⭐ For Lighthouse reporting that your production website is installable as PWA (except HTTPS).
⭐ For requesting a review and reviewing another team's PR.
That means you got 12/13 ⭐
Here is our implementation of the 7. excercise.
We also left this Review
Our website is also hosted and can be accessed here. The picture below shows the Lighthouse reporting which states that our production website is installable as PWA