-
Notifications
You must be signed in to change notification settings - Fork 415
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
refactor: improved navigation controls style #3410
Conversation
@@ -22,10 +22,10 @@ | |||
</div> | |||
<div class="navigation-controls"> | |||
<#if homepageLink?has_content> | |||
<div class="navigation-controls--btn navigation-controls--homepage" id="homepage-link" role="button"><a href="${homepageLink}"></a></div> | |||
<a class="navigation-controls--button navigation-controls--button_homepage" id="homepage-link" href="${homepageLink}" ></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@IgnatBeresnev I've just found out about header/footer html templates being used in user's projects and about corresponding problems caused by changes in html. So how bad is this change and should I revert it maybe for this time and butch all such changes to a special release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thought! The homepage button specifically hasn't been released yet, so it's perfectly fine and expected to change the styles related to it, see https://github.com/Kotlin/dokka/pull/3235/files#diff-316e3e9b3d50ff73dcc848844e5eaac72b2d38b7f1e84dca8a383a8b2c5a162c
As for other "unnecessary" / refactoring changes, I think it makes sense to minimize them in this release (1.9.20). We can break it down into two PRs: the first one fixes the issue in the most compatible way, we'll merge it and then have the code freeze at the end of the week. The second one introduces some additional changes from this PR, we'll merge it after code freeze. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I've rolled back unnecessary changes to the template
7fcfcff
to
b223f2a
Compare
b223f2a
to
ae78bf2
Compare
(cherry picked from commit 32d9815)
fixes #3300
Demo:
Also some html to test the result
test-page.zip