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

refactor: improved navigation controls style #3410

Merged
merged 1 commit into from Jan 12, 2024

Conversation

berezinant
Copy link
Contributor

fixes #3300

  • replaced icon
  • simplified css
  • fixed outline for navigation controls for better keyboard navigation

Demo:
Monosnap HelloKotlin 2023-12-12 23-03-32

Also some html to test the result
test-page.zip

@@ -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>
Copy link
Contributor Author

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?

Copy link
Member

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?

Copy link
Contributor Author

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

@berezinant
Copy link
Contributor Author

Fixed style for custom home page icon, e.g. github
image

@berezinant berezinant merged commit 32d9815 into master Jan 12, 2024
12 checks passed
@IgnatBeresnev IgnatBeresnev deleted the fix-3300-homepage-icon branch January 15, 2024 12:26
IgnatBeresnev pushed a commit that referenced this pull request Jan 15, 2024
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.

Verify CSS styles and the icon of the homepage link button
3 participants