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

Re-add footer with GoodJob version number #1201

Merged

Conversation

Pauloparakleto
Copy link
Contributor

@Pauloparakleto Pauloparakleto commented Dec 31, 2023

close #1183
Based on this discussion, it was suggested by @simi as an initial idea moving the footer message to the top.
image
@bensheldon About been not sure what to include in footer, I think should be useful author name(with link to author) and github(with link to goodjob repository).
image

Pauloparakleto and others added 9 commits December 31, 2023 15:31
Add secondary navbar with last updated data.
Render secondary navbar below navbar.
Add secondary_navbar translation.
Add border-bottom,
Remove footer class,
change the html element to a better context,
change id and data live poll region context. This is used in live_poll.js only. LivePoll.js take each data attribute to replace in the same place with new dom elements, so there is no risk to brake another page dynamism.
add secondary_navbar to all translations.
Not sure about what to include in footer? What about author name and link to github repository?
remove footer translation since it is simple author name and github. This is so to avoid needs of adding all translation.
# Conflicts:
#	app/views/good_job/shared/_footer.erb
#	app/views/good_job/shared/_secondary_navbar.erb
#	app/views/layouts/good_job/application.html.erb
#	config/locales/de.yml
#	config/locales/en.yml
#	config/locales/es.yml
#	config/locales/fr.yml
#	config/locales/ja.yml
#	config/locales/nl.yml
#	config/locales/ru.yml
#	config/locales/tr.yml
#	config/locales/uk.yml
@simi
Copy link
Contributor

simi commented Jan 3, 2024

I think footer can be removed at all then to release some space.

@Pauloparakleto
Copy link
Contributor Author

Was this merge intentional? @bensheldon ? I can rebase into new main and then push again mine to yours. The idea behind changing the translation files keys it to keep the context updated. Please, let me know what to do next. Happy new year!

@bensheldon
Copy link
Owner

@Pauloparakleto yep, sorry. I realized there was a duplicate feature in #1204, but I liked your retaining of the footer so I wanted to preserve that.

@bensheldon bensheldon changed the title Feat/1183 add secondary navbar Re-add footer with GoodJob version number Jan 3, 2024
@Pauloparakleto
Copy link
Contributor Author

@Pauloparakleto yep, sorry. I realized there was a duplicate feature in #1204, but I liked your retaining of the footer so I wanted to preserve that.

Thanks again, I have used this project a lot in my open source project. Contributing to yours has been part of my new job.

@bensheldon bensheldon added the enhancement New feature or request label Jan 3, 2024
@bensheldon bensheldon merged commit 0cadb9e into bensheldon:main Jan 3, 2024
20 checks passed
@bensheldon
Copy link
Owner

Thanks again, I have used this project a lot in my open source project. Contributing to yours has been part of my new job.

Yay! I love to hear that. I'm really glad that GoodJob has been of service! Also I really appreciate the contributions 🙇🏻

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

Successfully merging this pull request may close these issues.

Show last update on top nav bar
3 participants