www
/
www-new
12
2
Fork 3
Code Issues 56 Pull Requests 30 Projects Releases Wiki Activity
Labels Milestones

Add events archive #187

Merged
a3thakra merged 2 commits from adi-events-archive into main 2 years ago
Conversation 0 Commits 2 Files Changed 8
a3thakra commented 2 years ago
Collaborator

Closes #117

Closes #117
a3thakra requested review from j285he 2 years ago
j285he approved these changes 2 years ago
j285he left a comment
Collaborator

Looks great!

Looks great!
components/ArchivePage.tsx Outdated
import React from "react";
j285he commented 2 years ago
Collaborator

After the title, there's a missing border-bottom as per Figma

After the title, there's a missing border-bottom as per Figma
j285he commented 2 years ago
Collaborator

Also inspect element says the title is h1, should be h2

Also inspect element says the title is h1, should be h2
a3thakra commented 2 years ago
Poster
Collaborator

It should be h1 because it's the only heading of the page, so semantically it makes sense for it to be h1

It should be h1 because it's the only heading of the page, so semantically it makes sense for it to be h1
a3thakra marked this conversation as resolved
components/ArchivePage.tsx Outdated
import React from "react";
j285he commented 2 years ago
Collaborator

We have been using 60px for margin-bottom in [term], but I see 40px for this component. Maybe we change one or the other for consistency?

We have been using 60px for margin-bottom in [term], but I see 40px for this component. Maybe we change one or the other for consistency?
a3thakra commented 2 years ago
Poster
Collaborator

The line height is a lot more (3 instead of 1.5ish) than other sections, so imo 40px actually looks a lot like 60px. I tried to make it 60px and that was a lot.

The line height is a lot more (3 instead of 1.5ish) than other sections, so imo 40px actually looks a lot like 60px. I tried to make it 60px and that was a lot.
a3thakra marked this conversation as resolved
components/ArchivePage.tsx Outdated
: defaultGetShapesConfig(width, height);
}) as GetShapesConfig;
function capitalize(str: string) {
j285he commented 2 years ago
Collaborator

We're using this function a lot. Maybe it's worth putting it into lib?

We're using this function a lot. Maybe it's worth putting it into lib?
👍 1
a3thakra marked this conversation as resolved
a3thakra force-pushed adi-events-archive from 94c44b112d to 98717fafbc 2 years ago
a3thakra merged commit c31571f745 into main 2 years ago
a3thakra referenced this issue from a commit 2 years ago
Add events archive (#187)
a3thakra deleted branch adi-events-archive 2 years ago

Reviewers

j285he approved these changes 2 years ago
continuous-integration/drone/push Build is passing
Details
The pull request has been merged as c31571f745.
Sign in to join this conversation.
Reviewers
Request review
No reviewers
j285he
Labels
Apply labels
Clear labels
a11y Backlog Blocked Bug Content Dependencies Design Feature Request Good First Issue In Progress Performance Priority - High Priority - Low Priority - Medium Untriaged
No Label a11y Backlog Blocked Bug Content Dependencies Design Feature Request Good First Issue In Progress Performance Priority - High Priority - Low Priority - Medium Untriaged
Milestone
Set milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Assign users
Clear assignees
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#187
Write Preview
Loading…
Cancel
Save
There is no content yet.
Delete Branch 'adi-events-archive'

Deleting a branch is permanent. It CANNOT be undone. Continue?

No
Yes