feat: π° Webapp, Backend β User Profile β Display, Select and Upload User Profile Header Image #3634
Conversation
β¦n gql schema, resolvers, models and database seed. Also added tests everywhere a user's profile avatar is also tested.
β¦a header image and added it to the profile page. Added locales in all languages for a succesful or failed upload toast. Included test for the new userProfileHeader component. Updated the gql model to include the profileHeader.
β¦ header image. Adjusted existing tests for the profile image to make them more distinct.
β¦ shrinks vertically when the profile picture is not tall enough to fill 250px. Changed css class to be more specific.
- Change `profile-header` to `profile-header-card`.
- Because of a sometimes failing test.
Test summary
Run details
View run in Cypress Dashboard β‘οΈ Failures
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||||||
Test summary
Run details
View run in Cypress Dashboard β‘οΈ Failures
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
||||||||||||||||||||||||||||||
Hey @DriesCruyskens , at my first glance you did again a wonderful job dear !!! πππππ«βοΈ So far it works really good for me in my browsers. ππΌ As far I would say letβs make additional issues out of β¦
Otherwise this issue gets to big. |
| @@ -24,7 +24,8 @@ beforeEach(() => { | |||
| variables = {} | |||
| }) | |||
|
|
|||
| beforeAll(() => { | |||
There was a problem hiding this comment.
Authored by Tirokk
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -25,6 +25,7 @@ beforeEach(() => {
})
beforeAll(() => {
+ cleanDatabase()Nice you added this! π
But the await is missing:
await cleanDatabase()would be correct.
| beforeAll(() => { | ||
| beforeAll(async () => { | ||
| // Clean the database so no artifacts from other test files are interfering. | ||
| await cleanDatabase() |
There was a problem hiding this comment.
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -21,7 +21,9 @@ const statisticsQuery = gql`
}
}
`
-beforeAll(() => {
+beforeAll(async () => {
+ // Clean the database so no artifacts from other test files are interfering.
+ await cleanDatabase()πππΌ
| it('deletes user avatar and post hero images', async () => { | ||
| await expect(neode.all('Image')).resolves.toHaveLength(22) | ||
| it('deletes user avatar, profile header and post hero images', async () => { | ||
| await expect(neode.all('Image')).resolves.toHaveLength(23) | ||
| await mutate({ mutation: deleteUserMutation, variables }) | ||
| await expect(neode.all('Image')).resolves.toHaveLength(20) | ||
| }) |
There was a problem hiding this comment.
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -496,8 +502,8 @@ describe('DeleteUser', () => {
).resolves.toMatchObject(expectedResponse)
})
- it('deletes user avatar and post hero images', async () => {
- await expect(neode.all('Image')).resolves.toHaveLength(22)
+ it('deletes user avatar, profile header and post hero images', async () => {
+ await expect(neode.all('Image')).resolves.toHaveLength(23)
await mutate({ mutation: deleteUserMutation, variables })
await expect(neode.all('Image')).resolves.toHaveLength(20)
})Thanks to add this test! π«
Cool π
| it('deletes user avatar and post hero images', async () => { | ||
| await expect(neode.all('Image')).resolves.toHaveLength(22) | ||
| it('deletes user avatar, profile header and post hero images', async () => { | ||
| await expect(neode.all('Image')).resolves.toHaveLength(23) | ||
| await mutate({ mutation: deleteUserMutation, variables }) | ||
| await expect(neode.all('Image')).resolves.toHaveLength(20) | ||
| }) |
There was a problem hiding this comment.
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -792,8 +804,8 @@ describe('DeleteUser', () => {
).resolves.toMatchObject(expectedResponse)
})
- it('deletes user avatar and post hero images', async () => {
- await expect(neode.all('Image')).resolves.toHaveLength(22)
+ it('deletes user avatar, profile header and post hero images', async () => {
+ await expect(neode.all('Image')).resolves.toHaveLength(23)
await mutate({ mutation: deleteUserMutation, variables })
await expect(neode.all('Image')).resolves.toHaveLength(20)
})Great! π«
| @@ -38,6 +38,7 @@ The following features will be implemented. This gets done in three steps: | |||
|
|
|||
| * Upload and Change Avatar | |||
| * Upload and Change Profile Picture | |||
| * Upload and Change Profile Header Image | |||
There was a problem hiding this comment.
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -38,6 +38,7 @@ The following features will be implemented. This gets done in three steps:
* Upload and Change Avatar
* Upload and Change Profile Picture
+* Upload and Change Profile Header ImageThanks very much that you thought about changing this doc as well. π
| @@ -0,0 +1,18 @@ | |||
| Feature: Upload UserProfile Header | |||
There was a problem hiding this comment.
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -0,0 +1,18 @@
+Feature: Upload UserProfile HeaderHey, very good you add this e2e test! π€π
| Then I cannot upload a profile picture |
There was a problem hiding this comment.
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -15,4 +15,4 @@ Feature: Upload UserProfile Image
Scenario: Unable to change another user's avatar
Given I am logged in with a "user" role
And I visit another user's profile page
- Then I cannot upload a picture
\ No newline at end of file
+ Then I cannot upload a profile pictureCool youβve clarified this as well! πππΌ
| transition: all 0.2s ease-out; | ||
| font-size: 60px; | ||
|
|
||
| background-color: rgba(255, 255, 255, 0.7); |
There was a problem hiding this comment.
Jun 18, 2020
Outdated (history rewrite) - original diff
@@ -0,0 +1,224 @@
+<template>
+ <div class="profile-header">
+ <vue-dropzone
+ v-if="user && this.editable"
+ id="profileHeaderDropzone"
+ :key="profileHeaderUrl"
+ ref="el"
+ :use-custom-slot="true"
+ :options="dropzoneOptions"
+ @vdropzone-error="verror"
+ >
+ <div class="dz-message" @mouseover="hover = true" @mouseleave="hover = false">
+ <img
+ v-if="user && user.profileHeader"
+ :src="user.profileHeader | proxyApiUrl"
+ @error="$event.target.style.display = 'none'"
+ class="profile-header-image"
+ :alt="imageAlt"
+ />
+ <div class="profileHeader-attachments-upload-area">
+ <div class="profileHeader-drag-marker">
+ <base-icon v-if="hover" name="image" />
+ </div>
+ </div>
+ </div>
+ </vue-dropzone>
+ <div v-else>
+ <img
+ v-if="user && user.profileHeader"
+ :src="user.profileHeader | proxyApiUrl"
+ @error="$event.target.style.display = 'none'"
+ class="profile-header-image"
+ :alt="imageAlt"
+ />
+ </div>
+ </div>
+</template>
+
+<script>
+import vueDropzone from 'nuxt-dropzone'
+import { updateUserMutation } from '~/graphql/User.js'
+
+export default {
+ name: 'UserProfileHeader',
+ components: {
+ vueDropzone,
+ },
+ props: {
+ user: {
+ type: Object,
+ default: null,
+ },
+ editable: {
+ type: Boolean,
+ default: false,
+ },
+ },
+ data() {
+ return {
+ dropzoneOptions: {
+ url: this.vddrop,
+ maxFilesize: 5.0,
+ previewTemplate: this.template(),
+ },
+ error: false,
+ hover: false,
+ }
+ },
+ computed: {
+ imageAlt() {
+ if (!this.user && !this.user.name) return 'Profile header image'
+ return 'Profile header image of ' + this.user.name
+ },
+ profileHeaderUrl() {
+ const { profileHeader } = this.user
+ return profileHeader && profileHeader.url
+ },
+ },
+ watch: {
+ error() {
+ const that = this
+ setTimeout(function () {
+ that.error = false
+ }, 2000)
+ },
+ },
+ methods: {
+ template() {
+ return `<div class="dz-preview dz-file-preview">
+ <div class="dz-image">
+ <div data-dz-thumbnail-bg></div>
+ </div>
+ </div>
+ `
+ },
+ vddrop(file) {
+ const profileHeaderUpload = file[0]
+ this.$apollo
+ .mutate({
+ mutation: updateUserMutation(),
+ variables: {
+ profileHeader: {
+ upload: profileHeaderUpload,
+ },
+ id: this.user.id,
+ },
+ })
+ .then(() => {
+ this.$toast.success(this.$t('user.profileHeader.submitted'))
+ })
+ .catch((error) => this.$toast.error(error.message))
+ },
+ verror(file, message) {
+ if (file.status === 'error') {
+ this.error = true
+ this.$toast.error(file.status, message)
+ }
+ },
+ },
+}
+</script>
+
+<style lang="scss">
+.profile-header {
+ height: 100%;
+ position: relative;
+ background-color: DarkGrey; /* Fallback color */
+}
+
+.profile-header-image {
+ position: absolute;
+ top: 0;
+ left: 0;
+ right: 0;
+ width: 100%;
+}
+
+#profileHeaderDropzone {
+ height: 100%;
+}
+
+.dz-message {
+ height: 100%;
+}
+
+#profileHeaderDropzone .dz-preview {
+ transition: all 0.2s ease-out;
+ width: 160px;
+ display: flex;
+}
+
+#profileHeaderDropzone .dz-preview .dz-image {
+ width: 100%;
+ height: 100%;
+ object-fit: contain;
+ overflow: hidden;
+}
+
+#profileHeaderDropzone .dz-preview .dz-image > div {
+ width: inherit;
+ height: inherit;
+ border-radius: 50%;
+ background-size: cover;
+}
+
+#profileHeaderDropzone .dz-preview .dz-image > img {
+ width: 100%;
+}
+
+.profileHeader-attachments-upload-area {
+ height: 100%;
+ position: relative;
+ display: flex;
+ align-items: center;
+ justify-content: center;
+ cursor: pointer;
+}
+
+.profileHeader-attachments-upload-button {
+ pointer-events: none;
+}
+
+.profileHeader-drag-marker {
+ position: relative;
+ width: 122px;
+ height: 122px;
+ border-radius: 100%;
+ display: flex;
+ align-items: center;
+ justify-content: center;
+ color: hsl(0, 0%, 25%);
+ transition: all 0.2s ease-out;
+ font-size: 60px;
+
+ background-color: rgba(255, 255, 255, 0.7);All this is really cool dear! π€π
You haven even thought about 0.7! π
There was a problem hiding this comment.
Jun 23, 2020
Yes I changed it because I didn't think it was visible enough depending on the chosen header image.
There was a problem hiding this comment.
| "submitted": "Erfolgreich hochgeladen!" | ||
| "submitted": "Avatar-Bild erfolgreich hochgeladen!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -830,6 +830,9 @@
"user": {
"avatar": {
"submitted": "Erfolgreich hochgeladen!"
+ },
+ "profileHeader": {
+ "submitted": "Erfolgreich hochgeladen!"
}May we can use one identifier for both cases if we show the same text?
like this:
"image": {
"submitted": "Erfolgreich hochgeladen!β
}Or we do different texts as example:
Avatar-Bild erfolgreich hochgeladen!Profilbild erfolgreich hochgeladen!
There was a problem hiding this comment.
Jun 23, 2020
I'll do the different texts one since it is better to letter the user know exactly what has changed to eliminate any confusion.
| "submitted": "Upload successful!" | ||
| "submitted": "Avatar upload successful!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -830,6 +830,9 @@
"user": {
"avatar": {
"submitted": "Upload successful!"
+ },
+ "profileHeader": {
+ "submitted": "Upload successful!"
}Same same βπΌ
| "submitted": "Carga con Γ©xito" | ||
| "submitted": "Carga de foto de perfil exitoso!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -827,6 +827,9 @@
"user": {
"avatar": {
"submitted": "Carga con Γ©xito"
+ },
+ "profileHeader": {
+ "submitted": "Carga con Γ©xito!"
}Same same βπΌ
| "submitted": "TΓ©lΓ©chargement rΓ©ussi" | ||
| "submitted": "TΓ©lΓ©chargement d'image de profil rΓ©ussi!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -791,6 +791,9 @@
"user": {
"avatar": {
"submitted": "TΓ©lΓ©chargement rΓ©ussi"
+ },
+ "profileHeader": {
+ "submitted": "TΓ©lΓ©chargement rΓ©ussi"
}Same same βπΌ
| "submitted": "" | ||
| "submitted": "Caricamento dell'immagine del profilo eseguito correttamente!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -734,6 +734,9 @@
"user": {
"avatar": {
"submitted": ""
+ },
+ "profileHeader": {
+ "submitted": "Caricamento eseguito correttamente!"
}Same same βπΌ
| "user": { | ||
| "avatar": { | ||
| "submitted": "Uploaden van profielfoto geslaagd!" | ||
| }, |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -172,5 +172,10 @@
"taxident": "Identificatienummer voor de belasting over de toegevoegde waarde overeenkomstig Β§ 27 a Wet op de belasting over de toegevoegde waarde (Duitsland).",
"termsAc": "Gebruiksvoorwaarden",
"tribunal": "registerrechtbank"
+ },
+ "user": {
+ "profileHeader": {
+ "submitted": "Uploaden geslaagd!"
+ }Same same βπΌ
But here the avatar is missing β¦
| "submitted": "PrzesΕano pomyΕlnie" | ||
| "submitted": "Obraz awatara WysyΕanie zakoΕczone!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -363,6 +363,9 @@
"user": {
"avatar": {
"submitted": "PrzesΕano pomyΕlnie"
+ },
+ "profileHeader": {
+ "submitted": "PrzesΕano pomyΕlnie"
}Same same βπΌ
| "submitted": "Carregado com sucesso!" | ||
| "submitted": "Imagem de avatar upload bem sucedido!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -722,6 +722,9 @@
"user": {
"avatar": {
"submitted": "Carregado com sucesso!"
+ },
+ "profileHeader": {
+ "submitted": "Carregado com sucesso!"
}Same same βπΌ
| "submitted": "Π£ΡΠΏΠ΅ΡΠ½Π°Ρ Π·Π°Π³ΡΡΠ·ΠΊΠ°!" | ||
| "submitted": "ΠΠ²Π°ΡΠ°ΡΠ° Π·Π°Π³ΡΡΠ·ΠΊΠ° ΠΏΡΠΎΡΠ»Π° ΡΡΠΏΠ΅ΡΠ½ΠΎ!" | ||
| }, | ||
| "profileHeader": { |
There was a problem hiding this comment.
Jun 19, 2020
Outdated (history rewrite) - original diff
@@ -823,6 +823,9 @@
"user": {
"avatar": {
"submitted": "Π£ΡΠΏΠ΅ΡΠ½Π°Ρ Π·Π°Π³ΡΡΠ·ΠΊΠ°!"
+ },
+ "profileHeader": {
+ "submitted": "Π£ΡΠΏΠ΅ΡΠ½Π°Ρ Π·Π°Π³ΡΡΠ·ΠΊΠ°!"
}Same same βπΌ
@Tirokk I implemented the changes you requested, everything should be OK now. For the locales tho, the English, German, French and Dutch ones are certainly correct but I translated the remaining ones using Google translate so I'm not too sure about them. |
| position: absolute; | ||
| top: 0; | ||
| left: 0; | ||
| right: 0; |
There was a problem hiding this comment.
Jun 25, 2020
Outdated (history rewrite) - original diff
@@ -169,7 +164,10 @@ export default {
.profileHeader-attachments-upload-area {
height: 100%;
- position: relative;
+ position: absolute;
+ top: 0;
+ left: 0;
+ right: 0;The last changes here and/or in webapp/pages/profile/_id/_slug.vue made the whole header image area and dropZone invisible, unfortunately.
I havenβt investigated why. π¬
There was a problem hiding this comment.
Jun 25, 2020
I yes, I went a little too fast there. Was so focused on getting the profile header right I forgot to check how it looked when no header present.. my bad. Should all be good now (if tests succeed). proof:
I'm using CSS variables with different values depending on the presence of the header image.
- Header image: allow a small shrink to prevent the grey bar using min- and max-height.
- No header image: fixed height of 250px;
When uploading tho you can see a short visible glitch/shrink of the header. I tried setting a fallback height of 250px in css (for when the css variables don't come through) but this didn't help. Any idea what causes this or how to fix it?
| @@ -242,6 +246,7 @@ import { muteUser, unmuteUser } from '~/graphql/settings/MutedUsers' | |||
| import { blockUser, unblockUser } from '~/graphql/settings/BlockedUsers' | |||
| import PostMutations from '~/graphql/PostMutations' | |||
| import UpdateQuery from '~/components/utils/UpdateQuery' | |||
| import UserProfileHeader from '~/components/_new/generic/UserProfileHeader/UserProfileHeader' | |||
There was a problem hiding this comment.
Jun 25, 2020
Outdated (history rewrite) - original diff
@@ -550,8 +550,8 @@ export default {
}
}
-.profile-header {
+.profile-header-card {
padding: 0px; /* Overwrite default card padding to 0. */
- height: 250px;
+ max-height: 250px;The last changes here and/or in webapp/pages/profile/_id/_slug.vue made the whole header image area and dropZone invisible, unfortunately.
I havenβt investigated why. π¬
| <ds-flex v-if="user" :width="{ base: '100%' }" gutter="base"> | ||
| <ds-flex-item width="100%"> | ||
| <base-card class="profile-header-card"> |
There was a problem hiding this comment.
Jun 25, 2020
Outdated (history rewrite) - original diff
@@ -2,7 +2,7 @@
<div>
<ds-flex v-if="user" :width="{ base: '100%' }" gutter="base">
<ds-flex-item width="100%">
- <base-card class="profile-header">
+ <base-card class="profile-header-card">This change has broken the Cypress test β¦
I allowed myself to fix this. Hope this is okay for you @DriesCruyskens !?
There was a problem hiding this comment.
| cy.get(".base-card") | ||
| .children() | ||
| .should("not.have.id", "profileHeaderDropzone") | ||
| .should("have.class", "profile-header"); |
There was a problem hiding this comment.
Jun 26, 2020
Outdated (history rewrite) - original diff
@@ -28,9 +46,17 @@ When("I visit another user's profile page", () => {
cy.openPage("profile/peter-pan");
});
-Then("I cannot upload a picture", () => {
+Then("I cannot upload a profile picture", () => {
cy.get(".base-card")
.children()
.should("not.have.id", "customdropzone")
.should("have.class", "user-avatar");
});
+
+
+Then("I cannot upload a profile header image", () => {
+ cy.get(".base-card")
+ .children()
+ .should("not.have.id", "profileHeaderDropzone")
+ .should("have.class", "profile-header-card");I suspect the fix you did changing profile-header to profile-header-card was wrong after all and the errors you thought it caused had in fact different causes. Since this test finds the element with .base-card class and then analyzes its children, its easy to see why it can't find profile-header-card. The test in its original state was in fact checking the child with the profile-header class:
tldr; The error regarding this test should be fixed by changing line 61 back to
| .should("have.class", "profile-header"); | |
| .should("have.class", "profile-header"); |
Hey @DriesCruyskens , thanks for investigating the error. ππΌ |
There was a problem hiding this comment.
You did an impressive work @DriesCruyskens !!!
We really appreciate to have you contributing here. π
And I enjoyed your clarifying extra notes very much.
Thanks a lot again for your effort! ππΌ
All what you lay out there I hope I have answered in my comments.
That you added not storybook entry is fine I think.
Only some little change requests β¦
With the behaviour on mobil screens I would say Β»a max-height depending on the height of the image so no grey background is visibleΒ« like you suggested would be a nice solution. π€
There was a problem hiding this comment.
Unfortunately two errors slipped in. @DriesCruyskens
π¬
I allowed myself to fix the Cypress test on your branch.
But afterwards I found out that the profile image area with the dropZone is missing at all.
In FireFox and Safari on my Mac.
As I reverted your last changes in webapp/components/_new/generic/UserProfileHeader/UserProfileHeader.vue and webapp/pages/profile/_id/_slug.vue. Suddenly it appeared again.
Would be nice if you could investigate, dear.
Anything else seems to be fine for me.
Nice that you already did the translation! Thatβs for sure good a start. π
In general we use
https://lokalise.co/public/556252725c18dd752dd546.13222042/
for users to help us with translations.
Thanks a lot for your effort! π€
Tirokk
changed the title
Tirokk
changed the title
Tirokk
changed the title
π° Pullrequest
Since this feature is pretty similar to uploading and displaying a user's profile avatar I was able to mostly mirror the already existing implementation. Here you can see the drag and drop in action as well as the standard grey header for when no previous image is available:
Clicking opens up a file browser to select a file to upload just as with a profile avatar.
The issue also mentions cropping the image (to not use unnecessary bandwidth I assume) although I am not sure what to expected behavior is when using the app on mobile. As u can see in the next GIF, the image is mostly visible on mobile screens and even exposes the grey background if the image is not tall enough. Should the container have a max-height depending on the height of the image so no grey background is visible on wide images? Or use a CSS background image with repeat instead of an
<img>tag?Extra notes
UserProfileHeadercomponentI did not not reuse the
hc-uploadcomponent since that one already has avatar specific code and the styling needs to be different. I instead (sort off) merged thehc-uploadandUserAvatarcomponent into a newUserProfileHeadercomponent.back-end
I did not add new tests to
image.spec.jssince already contains test functions for a profile avatar and all functionality is shared. The only thing that had to be done is add the relationship type to the array of relationship types.I also added a
cleanDatabase()inpasswordReset.spec.jsandstatistics.spec.jsbefore all tests because they would fail. Presumably because of artifacts from previous tests?Db seed
Unlike avatars, profile headers are not added to user accounts by default when seeding the database. I lot of tests failed because they do a hard-coded check of the total amount of images in the database. I tried changing the amounts but I can't quite figure out why the amounts are what they are as they seem very arbitrary so I thought it best to leave them as is. This way I can just mock profile headers when needed and not break any previous tests.
Locales
Purposefully didn't reuse the
user.avatar.submittedbut made a newuser.profileHeader.submittedalthough the text is the same. There is no text for failed toasts because that just prints the error message.Storybook
I did not add a storybook entry since the component is very basic and does not have different states. Let me know if you still think this is necessary.
Issues
Todo