Skip to content
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

Feature | Add profile view #51

Merged
merged 32 commits into from
Jul 25, 2024
Merged

Feature | Add profile view #51

merged 32 commits into from
Jul 25, 2024

Conversation

EdgarB
Copy link
Collaborator

@EdgarB EdgarB commented Jul 17, 2024

Description

Ticket

How has this been tested?

Please mark the tests that you ran to verify your changes. If difficult to test, consider providing instructions so reviewers can test.

  • Manual testing
  • System tests
  • Unit tests
  • None

Checklist

  • CI pipeline is passing
  • My code follows the conventions of this project
  • I have performed a self-review of my code
  • I have commented on my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if applicable)
  • I have added seed data to the database (if applicable)

Release tasks

Add any tasks that need to be done before/after the release of this feature.

Screenshots/Loom

This section is relevant in case we want to share progress with the team, otherwise, it can be omitted.

@Sergio-e Sergio-e self-requested a review July 22, 2024 18:14
@LuigiR0jas LuigiR0jas self-requested a review July 22, 2024 20:49
@Sergio-e
Copy link
Collaborator

Not sure if the Figma was recently modified but the profile show page has several differences from Figma. Here are some:

  • Content isn't centered
  • "Edit profile" should be a link and not a button
  • Name needs to be in italics
  • I suggest using "Anonymous" instead of "No name"
  • I suggest removing "No location" and "No description"
  • I suggest using a grey background and a 'cursor-not-allowed' class if the user didn't filled a social link. For example, if a user only added a GitHub url and no LinkedIn or Twitter url.

image
image

@Sergio-e
Copy link
Collaborator

Here are some comments related to the profile edit page:

  • Let's limit the width similar to how the registration page limits the form size when the screen is too large
  • I suggest using the same styling as the text fields in the registration and session pages
  • Add a border between "Mail notifications" and "In-app notifications"

image
image

@EdgarB
Copy link
Collaborator Author

EdgarB commented Jul 23, 2024

Figma was recently updated and the required changes on the tailwind configuration weren't up to date.
I'll make the corresponding changes on the next commits.
Thanks for the feedback @Sergio-e.

app/views/profiles/show.html.erb Show resolved Hide resolved
app/views/profiles/show.html.erb Outdated Show resolved Hide resolved
app/views/profiles/edit.html.erb Outdated Show resolved Hide resolved
@Sergio-e
Copy link
Collaborator

I just noticed that we forgot to add a way to allow users to remove an image if they already uploaded one. It would be nice if you and Charlie could come up with a simple way to solve it
image

@Sergio-e
Copy link
Collaborator

I'm unable to remove images. I'm seeing the following after clicking on the "-" icon
image

@Sergio-e
Copy link
Collaborator

Seems like the icon should be on the upper right corner instead of on the lower left
image

@Sergio-e
Copy link
Collaborator

let's hide the "-" icon if the user hasn't uploaded an image

@Sergio-e Sergio-e merged commit 904c0aa into main Jul 25, 2024
4 checks passed
@Sergio-e Sergio-e deleted the feature/new-profile branch July 25, 2024 22:34
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.

2 participants