Fix title inconsistencies #229

Merged
j285he merged 5 commits from jared-fix-headers into main 2021-08-30 20:49:13 -04:00
Member

Closes #212.

Closes #212.
j285he added 1 commit 2021-08-30 19:00:39 -04:00
continuous-integration/drone/push Build is passing Details
b9f0d81c48
Fix inconsistencies
j285he added 1 commit 2021-08-30 19:11:57 -04:00
continuous-integration/drone/push Build is passing Details
aba9a1bb4c
Separate the issues
j285he changed title from Fix header inconsistencies to Fix title inconsistencies 2021-08-30 19:12:22 -04:00
a3thakra reviewed 2021-08-30 19:13:37 -04:00
@ -4,1 +4,4 @@
.page > h1 {
padding-bottom: calc(16rem / 16);
border-bottom: 1px solid var(--primary-heading);
Owner

calc(1rem / 16) solid ...

calc(1rem / 16) solid ...
j285he marked this conversation as resolved
a3thakra approved these changes 2021-08-30 19:14:02 -04:00
Dismissed
a3thakra left a comment
Owner

just 1 nit, lgtm otherwise

just 1 nit, lgtm otherwise
j285he added 1 commit 2021-08-30 19:16:24 -04:00
continuous-integration/drone/push Build is passing Details
e491a4076d
Pesky pixels
a3thakra reviewed 2021-08-30 19:16:32 -04:00
@ -67,3 +67,3 @@
{hasFutureEvents && (
<>
<h2>Upcoming Events</h2>
<h1>Upcoming Events</h1>
Owner

actually can you put this in a section tag? technically, there are two h1's on the page. But a page cannot have 2 h1s. If we look closely, we have two sections on this page, each with it's own h1. I think that would make this nicer. (in terms of semantics)

actually can you put this in a section tag? technically, there are two h1's on the page. But a page cannot have 2 h1s. If we look closely, we have two sections on this page, each with it's own h1. I think that would make this nicer. (in terms of semantics)
Author
Member

I currently have Past Events as an h2. Should I make it an h1?

I currently have Past Events as an h2. Should I make it an h1?
Owner

Yes, semantically speaking, each page should have exactly one h1, and then 0 or more h2's

Yes, semantically speaking, each page should have exactly one h1, and then 0 or more h2's
Owner

when i say h1, i mean the h1 tag. you can style it as you wish.

when i say h1, i mean the h1 tag. you can style it as you wish.
Author
Member

Ah okay. Also for the original comment, I don't see two h1's on the same page. It's just either Upcoming Events or Events Archive:. Am I going crazy?

Ah okay. Also for the original comment, I don't see two h1's on the same page. It's just either `Upcoming Events` or `Events Archive:`. Am I going crazy?
Owner

"Upcoming events" and "Past events" should both be h1s enclosed within a section. So the final markup would look something like this:

<section>
    <h1>Upcoming events</h1>
    {events.map(...)}
</section>
<section>
    <h1>Past events</h1>
    {events.map(...)}
</section>
"Upcoming events" and "Past events" should both be `h1`s enclosed within a `section`. So the final markup would look something like this: ```html <section> <h1>Upcoming events</h1> {events.map(...)} </section> <section> <h1>Past events</h1> {events.map(...)} </section> ```
Author
Member

Ahhhhhh sorry I see what you mean, thanks!

Although what are your thoughts on the styling? I think having h2 on Past Events makes the user more drawn towards current upcoming events, instead of having two blocky h1's competing for attention, but idk

Ahhhhhh sorry I see what you mean, thanks! Although what are your thoughts on the styling? I think having h2 on Past Events makes the user more drawn towards current upcoming events, instead of having two blocky h1's competing for attention, but idk
Owner

As I said earlier, they should be h1 tags, you can style them like h2s to match figma 😉

As I said earlier, they should be h1 tags, you can style them like h2s to match figma 😉
Author
Member

OK, thank you! Since Figma's title sizes differ a bit from ours, let me know if I should change the size of Past Events or not

OK, thank you! Since Figma's title sizes differ a bit from ours, let me know if I should change the size of Past Events or not
a3thakra dismissed a3thakra’s review 2021-08-30 19:17:29 -04:00
Reason:

create two sections on the events/term/index page.

j285he added 1 commit 2021-08-30 20:02:26 -04:00
continuous-integration/drone/push Build is passing Details
87b5390a50
Add sections
a3thakra approved these changes 2021-08-30 20:29:46 -04:00
a3thakra left a comment
Owner

LGTM

LGTM
j285he added 1 commit 2021-08-30 20:43:49 -04:00
continuous-integration/drone/push Build is passing Details
30a0423adc
Merge main
j285he merged commit 485230c95a into main 2021-08-30 20:49:13 -04:00
j285he deleted branch jared-fix-headers 2021-08-30 20:49:19 -04:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#229
No description provided.