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

Closes #322: Feature request: Clickable logo that points to specific url #325

Merged
merged 2 commits into from
Aug 17, 2017

Conversation

khorolets
Copy link
Contributor

@khorolets khorolets commented Aug 16, 2017

Summary:

  • Feature request: Clickable logo that points to specific url #322: Feature request: Clickable logo that points to specific url
  • Add "Return to website" link to the top right that leads to info.contacts.url if any.
  • Increase padding top for .api-info-wrapper when left sidebar is hiding to avoid header overlaying by top menu.

@RomanGotsiy Unfortunately I don't do Angular, so I'm not sure if lib/components/ApiInfo/api-info.html is correct.

I, also, added a "Return to website" button, I need it, maybe someone need it, too. (It won't show up if no url in contact section of spec is present. Also, we can hide it with css .go-back-link { display: none })

And I've noticed that .api-info-wrapper h1 becomes overlaid by top menu on small devices (once left sidebar is hiding), so I've increased the top padding for that.

I haven't found contributors rules in the repo, so I'm not sure if it is OK to do so. So let me know if anything is wrong and needs to be edited.

Thanks.

UPDATE

Forgot to attach screenshot of the results:
redoc_return_link

@RomanHotsiy
Copy link
Member

Hey @khorolets! Thanks for PR!
Great idea to use info.contacts.url!

I have concerns abot "return to website" button. I think it shouldn't be shown by default. I'm sure some ReDoc users don't need (and don't want) this button as they just use navigation bar on the top (for example https://apis.guru/api-doc/)
Also I don't like position of this button. Let's track this feature request separatelly. You can open another PR for this.

Regarding your padding fix, could you just put it into a separate commit with fix: prefix in message (it is neaded for auto changelog generation)
Thanks!

@khorolets
Copy link
Contributor Author

@RomanGotsiy ok, thanks. I will update my PR in 12 hours or so.

@hlorofos
Copy link

By adding hardcoded string "return to website" we are limiting some users who want to use different language for example.

@khorolets khorolets force-pushed the issue-322_clickable_logo branch from 51b5626 to 7e5267f Compare August 17, 2017 13:57
@khorolets
Copy link
Contributor Author

@hlorofos Totally agreed. Removed that change.
@RomanGotsiy Updated according to your request. Please, verify.

@RomanHotsiy RomanHotsiy merged commit 514fc29 into Redocly:master Aug 17, 2017
@RomanHotsiy
Copy link
Member

@hlorofos Great point! 👍

@khorolets awesome! Merged! Thanks so much!!!

I will release a new release at the beginning of the next week

@khorolets
Copy link
Contributor Author

@RomanGotsiy Thank you!

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.

3 participants