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

CAMEL-15392: documentation and community redesign #471

Merged
merged 1 commit into from Oct 26, 2020
Merged

Conversation

zregvart
Copy link
Member

Based on pull request #469 by @aashnajena, rebased with fixed links.

Copy link
Contributor

@aashnajena aashnajena left a comment

Choose a reason for hiding this comment

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

LGTM

@AemieJ
Copy link
Contributor

AemieJ commented Aug 28, 2020

issue-prob

This is an issue within the documentation page for the certain screen width. line-height must be provided to overcome this.

@AemieJ
Copy link
Contributor

AemieJ commented Aug 28, 2020

I personally find the structure of the documentation page within the desktop screen version quite disorientated and unstructured. I would prefer it if the images were on the same side as presented as on the community page, looks neater. Also, I observed that due to one image for camel-core the context isn't aligned with the contents of the sub-project.

I find the following structure much neater:
structured-look

display: inline-block;
box-shadow: 0 4px #8e480b;
font-weight: bold;
padding: 0.4rem 1rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
padding: 0.4rem 1rem;
padding: 0.4rem 1rem;
line-height: 2.5rem;

Copy link
Contributor

Choose a reason for hiding this comment

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

When exactly are you observing the overlap? I have already provided line-height for <1024px screens (line 355) to avoid the overlap, and I can't see the buttons overlapping for any screen width.

Copy link
Contributor

Choose a reason for hiding this comment

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

This overlapping is observed for 1024px as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

My view at 1024px 100% zoom

Screenshot 2020-08-28 at 1 48 47 PM

@aashnajena
Copy link
Contributor

@AemieJ The camel core context is not aligned with the rest because there's only one icon present, and if we give it 30% width, it leaves a lot of blank space which looks bad. Since the alignments are alternating, it's not a noticeable flaw.

I did think about making them all left-aligned, but it looks very odd to have one icon for camel core and two for the rest. If we can use just one icon for each sub-project, the left-alignment will look good.

@AemieJ
Copy link
Contributor

AemieJ commented Aug 28, 2020

@aashnajena I hear you but I respectfully disagree with you. In my opinion, altering a few lines of CSS and making it look structured and consistent through gives a neater effect to the design be it 1 or 2 images.

.docs .box .content,
.community .box .content,
.docs .box .icon,
.community .box .icon {
Copy link
Contributor

Choose a reason for hiding this comment

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

Missed out on these rules earlier

Suggested change
.community .box .icon {
.community .box .icon,
.docs .box.camel-core .content,
.docs .box.camel-core .icon {

display: inline-block;
box-shadow: 0 4px #8e480b;
font-weight: bold;
padding: 0.4rem 1rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

My view at 1024px 100% zoom

Screenshot 2020-08-28 at 1 48 47 PM

@aashnajena
Copy link
Contributor

@zregvart can we fix and merge this? The website still shows underlines under images and I think this PR fixes that as well

@zregvart
Copy link
Member Author

@aashnajena thanks for the ping. I'll rebase and merge this later today.

@zregvart
Copy link
Member Author

I'm rather tied up with other tasks, so I didn't have the time to look at this yesterday, I'll try to find some time today.

Based on pull request #469 by @aashnajena, rebased with fixed links.
@zregvart zregvart merged commit 338b9b0 into master Oct 26, 2020
@zregvart zregvart deleted the pr/docs-layout branch October 26, 2020 15:52
@zregvart
Copy link
Member Author

This is now merged thanks @aashnajena and @AemieJ. I'm sure we'll find other issues to improve, feel free to raise issues or create additional PRs for them.

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.

None yet

3 participants