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

Right rail content #502

Merged
merged 41 commits into from
Jul 30, 2020
Merged

Right rail content #502

merged 41 commits into from
Jul 30, 2020

Conversation

jerelmiller
Copy link
Contributor

Closes #476

Description

Adds a "right rail" component that houses the area to contribute to the project. The right rail only shows up on guide pages.

Screenshot(s)

localhost_8000_collect-data_collect-data-from-any-source (1)

@jerelmiller jerelmiller added the enhancement New feature or request label Jul 30, 2020
Copy link
Contributor

@zstix zstix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment about the underline.

Also, would it make sense to move src/layout/MainLayout.js to the src/components/PageLayout directory? The distinction between the two isn't super clear to me at the moment.

Lastly, I feel like the guide content gets a little squished at certain breakpoints. It probably doesn't help that this is the exact breakpoint that I use in FireFox:

Screen Shot 2020-07-30 at 10 13 54 AM
Screen Shot 2020-07-30 at 10 14 11 AM

Not sure if @djsauble has any opinions on this.

<PageTitle>Observability for every developer</PageTitle>
<SEO />
<PageLayout type={PageLayout.TYPE.SINGLE_COLUMN}>
<PageLayout.Header title="Observability for every developer" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds an underline to the title on the homepage, which doesn't currently exist. We might want to add a lil css override for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm actually not seeing the underline for most of our pages. We may just want to remove this an add it where applicable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the underline intentionally on every page, including the homepage, so that every page had a consistent page title treatment. I might need to chat with @djsauble one more time and get his thoughts 😄

@jerelmiller
Copy link
Contributor Author

Also, would it make sense to move src/layout/MainLayout.js to the src/components/PageLayout directory?

Not quite. I can understand the confusion, which is why I had such a hard time trying to name the PageLayout component. The MainLayout is meant to represent the layout of the entire website (global header, side nav, main content, footer). The PageLayout is meant to represent the layout only of the content area (i.e. the "page" content). Right now this consists of 2 layouts: single column (overview pages, home page, etc) and related content (2 column with content area and the related content area).

We are going to want to add that related content area to more pages over time, so the thinking was to try and make this easy when we come back to this for other pages. Ideally its a switch of the type prop to the PageLayout, and the addition of the PageLayout.RelatedContent to the page.

From a developer point of view, I'd love to get your thoughts on this API and by all means, if you have a better name that PageLayout, I'm all for it. I'm not totally in love with it, but its the best I had 😄 .

@jerelmiller
Copy link
Contributor Author

@djsauble I think we need to figure out how we can make the content area bigger. That or maybe we need a better breakpoint for when the 2-column layout switches to a single column in the content area.

@jerelmiller
Copy link
Contributor Author

@zstix After talking with @djsauble we decided to keep the underline on every page to maintain consistency with the separation of the page title and the content. I've also pushed up some changes to 1) widen the layout by 100px to help those with super-wide monitors and 2) update the breakpoint that switches the side by side content to single column. That should help people with mid-size screens see both the related content and main content area without making it feel squished. Take a look and see if this works a bit better for you!

Copy link
Contributor

@zstix zstix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really nice, thank you for for tweaking that!!!

@jerelmiller jerelmiller merged commit 041a1fa into main Jul 30, 2020
@jerelmiller jerelmiller deleted the jerel/right-rail branch July 30, 2020 23:01
@nr-opensource-bot
Copy link
Contributor

🎉 This PR is included in version 1.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Related Content] Create "right rail" component (UI only)
3 participants