Story 2304: User Profile UI#2400
Conversation
julioest
left a comment
There was a problem hiding this comment.
Looks great and works well in both states! One small thought: we could drop that duplicate markup (achievements, badges, posts, connections) and let CSS grid do the layout work for us instead.
I tried collapsing it into a single grid with grid-template-areas first, but every row took the height of the taller card across columns and left big gaps under the shorter ones. Two flex columns with display: contents on mobile cleared both problems up: each card renders once, the columns flow independently, and the mobile cascade is just order:.
<div class="user-profile__profile-content-area">
<div class="user-profile__col">
{# bio, github, mailing-list #}
</div>
<div class="user-profile__col">
{# achievements, badges, posts, connections #}
</div>
</div>.user-profile__profile-content-area {
display: grid;
grid-template-columns: 1fr 1fr;
gap: var(--space-large);
margin-bottom: var(--space-xl);
}
.user-profile__col {
display: flex;
flex-direction: column;
gap: var(--space-large);
}
@media (max-width: 767px) {
.user-profile__profile-content-area {
grid-template-columns: 1fr;
}
/* Lift the column wrappers so the cards become direct grid children */
.user-profile__col {
display: contents;
}
.user-profile__bio { order: 1; }
.user-profile__achievements { order: 2; }
.user-profile__badges { order: 3; }
.user-profile__github { order: 4; }
.user-profile__posts { order: 5; }
.user-profile__mailing-list { order: 6; }
.user-profile__connections { order: 7; }
}Tried it locally and each card class shows up once, columns flow independently on desktop, and the mobile cascade orders bio → achievements → badges → github → posts → mailing-list → connections.
Here's a patch for this: pr-2400-dom-duplication-fix.patch
This is slick as heck, implemented. |
837cfbf to
ebee7e1
Compare
herzog0
left a comment
There was a problem hiding this comment.
Looking pretty good! Just a few fixes I think should be applied. Also, can you add some info to the description on how we can see other user's public profiles? I see me in the url, but I wouldn't know what to replace that with in order to see others' users pages.
| {% include "v3/includes/_achievement_card.html" with achievements=achievements_data.achievements primary_button_url="https://www.example.com" %} | ||
| </div> | ||
| <div class="user-profile__badges"> | ||
| {% include "v3/includes/_badges_card.html" with cta_url="#" cta_label="Explore available badges and how to earn them" empty_icon_srcs=badge_icon_srcs badges=demo_badges %} |
There was a problem hiding this comment.
we're passing empty_icon_srcs=badge_icon_srcs here but empty_icon_srcs is not defined in the badges card component, nor badge_icon_srcs is defined in the view context
There was a problem hiding this comment.
That's what I get for trying to fix the card changes mid meeting! Thanks for the catch.
| {% block content %} | ||
| <div class="user-profile__container"> | ||
| <div class="user-profile__profile-card-row"> | ||
| {% with user_info as user %} |
There was a problem hiding this comment.
Is there any different between with user_info as user and with user_info=user? I'm seeing two different patterns here, just wondering if it's purposeful.
There was a problem hiding this comment.
Those are equivalent statements, this is just an older style of writing the same thing! I'm happy to change it if we decide we want the more modern style for consistency. Reference for this tag: https://docs.djangoproject.com/en/6.0/ref/templates/builtins/#with
|
|
||
| .btn-secondary { | ||
| border-color: var(--color-stroke-strong); | ||
| background-color: var(--color-surface-weak); |
There was a problem hiding this comment.
Sorry, is this actually needed? Cause we have audited the foundational buttons already and it feels unexpected to me to see any changes in them at this point
There was a problem hiding this comment.
I believe it is necessary. Without this the non hero version of secondary buttons had no background color, so it picked up the grey behind it. I think this may have slipped through the cracks, since the background color on the demo page is the correct background color for the secondary buttons?
92a4212 to
d5e1bc2
Compare
…e conflict, remove extra argument in user_profile_page.html
Issue: #2304
Summary & Context
Changes
_user_profile_bio_cardcomponent, which is a variation of a markdown card with space of library contributions_mailing_list_activitycomponent, which lists mailing list activity on a card component.summaryto_post_cardfor use with the empty statePlease list any potential risks or areas that need extra attention during review/testing
Screenshots
Empty:
Desktop:
Dark Mode:

Mobile:

Filled:
Desktop:
Mobile:

Self-review Checklist
Frontend