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

New social buttons on navbar V2 #864

Merged
merged 9 commits into from
Feb 2, 2022
Merged

New social buttons on navbar V2 #864

merged 9 commits into from
Feb 2, 2022

Conversation

fjtc
Copy link
Contributor

@fjtc fjtc commented Feb 2, 2022

What this PR does / why we need it:

Implements the features describe in the issue #862 by adding any number of buttons just like repo_button. This is an example of what this new feature can do:

image

[repo_button]
  url = "https://github.com/okkur/syna"
  text = "Star" # default: "Star"
  icon = "fab fa-github" # defaults: "fab fa-github"

[[buttons]] 
  url = "https://linkedin.com/"
  title="LinkedIn"
  text=""

[[buttons]] 
  url = "https://facebook.com/"
  title="Facebook"

[[buttons]] 
  url = "https://about.okkur.org/"
  title="Okkur Labs"
  text = ""
  icon = "fas fa-home"    

It replaces the previous PR #863 with the adjusts suggested by @stp-ip:

  • It can replace or be used in conjunction with repo_button, so it will not break backward compatibility;
  • Is no longer fixed to a set of predefined buttons;
  • Is way more configurable, allowing the definition of any number of buttons with icon+text and icon;

This patch also includes a new parameter repo_button.no_text that allows it to be used with no text at all. This parameter has been added to make it easier to display the repo_button with the icon centered inside the button.

Special notes for your reviewer:

Both the template and the documentation were updated to fully display the capabilities of this patch.

Release note:

- Adding one or more customizable buttons to the nav bar that can be used just like the repo_button or replace it;
- The nav bar repo_button now has the optional parameter no_text that removes the text label and centers the icons inside the button;
- Adding 2 new translation strings to pt.yaml that are used by repo_button;

Copy link
Member

@stp-ip stp-ip left a comment

Choose a reason for hiding this comment

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

Awesome. That is a lot cleaner and fits in well. Like the reuse of the button fragment and the addition of icons to them as well.

Merging and looking forward to the next idea :)

@stp-ip
Copy link
Member

stp-ip commented Feb 2, 2022

Sidenote: I will squash merge this so it is a clean single commit.
One thing that might be a nice option is using git -p and then restructure a PR into the commit steps you feel are worthwhile information. Say in this case adding the icons to the button fragment could be one, the next could be the usage of button within the nav and the last translation additions for example.

@stp-ip stp-ip merged commit 36d3ec8 into okkur:master Feb 2, 2022
@fjtc
Copy link
Contributor Author

fjtc commented Feb 2, 2022

Thanks for the suggestions. As a new Hugo user, your suggestions and guidance are invaluable to us. I'm pretty sure we will contribute more to the project as we found Syna a perfect match to our needs.

I'm planning to send another patch soon with some enhancements we made to the button fragment. I noticed that we sent a partial implementation of it without the proper documentation.

Thanks to keep such a nice theme open for everyone to use.

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

Successfully merging this pull request may close these issues.

2 participants