Skip to content

Create Footer component (#499) #534

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jul 25, 2023
Merged

Create Footer component (#499) #534

merged 4 commits into from
Jul 25, 2023

Conversation

@yassa-tamer
Copy link
Contributor

@yassa-tamer yassa-tamer commented Jul 20, 2023 •
edited by justin808
Loading

This commit basically should add a footer to the Simple React page, But The footer logic is used in more than one place so a Footer component is created and called wherever it is needed.

Resolves #499

This change is Reviewable

@yassa-tamer
Create Footer component (shakacode#499)
df0ca97 
This commit basically should add a footer to the Simple React page,
But The footer logic is used in more than one place so
a Footer component is created and called wherever it is needed.

Resolves shakacode#499
ahangarha
ahangarha suggested changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@ahangarha ahangarha left a comment

Choose a reason for hiding this comment

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

Good move

I see duplicate content at the bottom of the page.

image

What do you think about the Classic Rails page? What to do there?

If we can have the same navigation bar on all pages, we should be able to have a footer too.

@yassa-tamer
Copy link
Contributor Author

Yes, Sorry for forgetting it. I removed the duplicate content.
If we want the footer on every page, I suggest putting it on the application layout with the navigation bar.

@yassa-tamer
Move Footer to application layout
a931337 
The Footer should appear in all the pages.
ahangarha
ahangarha suggested changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@ahangarha ahangarha left a comment

Choose a reason for hiding this comment

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

Looks good to me.

There are minor issues to fix. After that, I think this PR is ready to get merged.

ahangarha
ahangarha reviewed Jul 21, 2023
View reviewed changes
ahangarha
ahangarha approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@ahangarha ahangarha left a comment

Choose a reason for hiding this comment

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

Good job 👍🏾

@ahangarha
Copy link
Contributor

@justin808 Now we have a footer on all pages. Do you have any comments here?

I think we need to update the branding in the footer. Also, the UI/UX can get improved.
I propose to have a separate task for these two.

@justin808 justin808 merged commit 97c1ac5 into shakacode:master Jul 25, 2023
@justin808
Copy link
Member

Thanks @Yassa-hue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@ahangarha ahangarha ahangarha approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Tab "Simple React" doesn't have footer

3 participants

@yassa-tamer @ahangarha @justin808