-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add project images #535
Add project images #535
Conversation
# Conflicts: # src/app/config/wizard-page-config.ts # src/app/models/resources/project-add.ts # src/app/modules/project/add/add.module.ts # src/app/services/wizard.service.ts
# Conflicts: # src/app/modules/project/details/details.component.scss
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks alot for another nice new feature Walter! 🥇
I had some remarks.
-
For some reason when switching from homepage to project overview page it took like 9s to load all projects for me. Anyone else experienced this? (edit: after reboot it didn't happen anymore)
-
I would suggest to replace the uploading image wizard page step to be after the upload project icon step.
-
I like the new way of uploading a project icon in the edit page. But I think it may be a bit unclear how to delete it. I suggest we create a new issue for it if this would take too much time.
-
On the edit project page a loading icon was permanently showing in the edit project button. I think this spinner is nicer than the one we use on other pages 👍
-
Do you guys think we should give the carrousel buttons a different color? In this case they are not very visible.
-
We could improve on how pictures are shown. For example this Delta icon isn't readable when uploaded. I guess we can create a new issue for that.
…into feature/464-add-project-images
@niraymak i added a small dropshadow to combat the white background issue Also i changed the carousel to stay in a 16:9 aspect ratio so the image looks the same on all devices |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! Looks good
Description
Project images were planned for a long time in the detail page, this pull request will implement both adding and showing the project images.
Type of change
Checklist
Steps to Test or Reproduce
Outline the steps to test or reproduce the PR here.
These steps will be used during release testing.
Link to issue
Closes: #464
Closes: #539