Skip to content

feat: Create profile page #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 29, 2021
Merged

feat: Create profile page #2

merged 7 commits into from
Jul 29, 2021

Conversation

fitzhavey
Copy link
Member

@fitzhavey fitzhavey commented Jul 27, 2021

This PR will scaffold the Profile page. It won't be able add all the functionality as we'll need to create an API to allow us to:

  • confirm summoner names (by querying the riot api)
  • set a summoner for a user account to follow

It also adds the logic to check that an account is setup, and if not to redirect them to the profile page.
Screenshot 2021-07-29 at 00 13 48

@fitzhavey fitzhavey changed the title chore: scaffold profile page DRAFT~ chore: scaffold profile page Jul 27, 2021
@fitzhavey fitzhavey changed the title DRAFT~ chore: scaffold profile page feat: Create profile page Jul 28, 2021
@fitzhavey fitzhavey requested a review from andrewlew1s July 28, 2021 23:15
Comment on lines +2 to +3
dist/**/*.js
dist/**/*.js.map
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant to the profile page. This was renaming the cloud functions typescript build output to dist, as lib is a name commonly used in our architecture.

@@ -12,7 +12,7 @@
"engines": {
"node": "12"
},
"main": "lib/index.js",
"main": "dist/index.js",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, relating to the cloud functions build naming

@@ -3,7 +3,7 @@
"module": "commonjs",
"noImplicitReturns": true,
"noUnusedLocals": true,
"outDir": "lib",
"outDir": "dist",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, relating to the cloud functions build naming

@@ -14,6 +14,6 @@ exports.onAccountCreation = functions.auth.user().onCreate(async (user) => {
const usersCollection = admin.firestore().collection('users')
await usersCollection.doc(uid).set({
email: email,
summoners: [],
summonerId: null,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured it was simpler to have a single summonerId for now and support multiple summoners later on

@@ -21,7 +21,11 @@ export default {
},

router: {
middleware: ['auth'],
middleware: ['auth', 'accountSetup'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how we enable the accountSetup middleware to run before the user can access a page

},

auth: {
redirect: { home: false },
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is weird, we aren't really using Nuxt's built in auth solution as far as I know. We were getting an error on the account setup redirect and this fixes is, as per:
nuxt-community/auth-module#731 (comment)

@@ -66,6 +70,7 @@ export default {
onAuthStateChangedAction: 'account/onAuthStateChanged',
},
},
firestore: true,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple enabling firestore database access 🔥

<h1>rito-plz</h1>
</section>
</div>
<section class="Home">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did some layout simplification on all the pages too, they all had an extra wrapping div that could be removed

},
},
created() {
;(this as any).fetchData()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gross workaround for typescript issue. Vue methods aren't available in the created hook:

vuejs/vue#8721 (comment)

},
async fetchData() {
this.isLoading = true
await (this as any).fetchUser({ id: this.user.uid })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same workaround as above since vuex actions aren't predefined on the vue instance

@fitzhavey fitzhavey merged commit a5c56d9 into main Jul 29, 2021
@fitzhavey fitzhavey deleted the feat/profile-page branch July 29, 2021 07:37
@fitzhavey fitzhavey restored the feat/profile-page branch July 29, 2021 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant