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

Visually fix the focus indicator by adjusting padding #192

Merged
merged 1 commit into from
Jan 14, 2023

Conversation

dougwaldron
Copy link
Contributor

When navigating a page by keyboard, the browser draws an outline around the currently focused link. When an AnchorJS link is focused, the outlined icon looks awkward because the padding applied is unbalanced.

This pull request adjusts the left and right padding to be equal and also adjusts the margin to maintain the default icon placement. Tests have been updated to match the changes, and all tests are passing. Several of the examples in the documentation also had to be fixed.

Here are screenshots of the focus indicator before and after these changes:

Before

image image

After

image image

@bryanbraun
Copy link
Owner

Ooh, very interesting! This will take a bit of testing but I think it's a great improvement and I'm looking forward to getting this fixed.

I did some small docs updates that unfortunately caused some conflicts. If you have some time to resolve them, that would be helpful. Otherwise, I may be able to take a pass at it next week.

@dougwaldron
Copy link
Contributor Author

I did some small docs updates that unfortunately caused some conflicts. If you have some time to resolve them, that would be helpful. Otherwise, I may be able to take a pass at it next week.

Yeah, no problem, I'll get it fixed up.

Copy link
Owner

@bryanbraun bryanbraun left a comment

Choose a reason for hiding this comment

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

I did a lot of checking and this is looking great. Merging!

display: block;
position: absolute;
top: -5px;
left: 5px;
Copy link
Owner

Choose a reason for hiding this comment

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

Wow, thanks for catching this one that I overlooked in my last commit!

@bryanbraun bryanbraun merged commit e4619d1 into bryanbraun:master Jan 14, 2023
@dougwaldron
Copy link
Contributor Author

I did a lot of checking and this is looking great. Merging!

Cool, thanks!

NotMyFault referenced this pull request in jenkins-infra/jenkins.io Jan 25, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [anchor-js](https://www.bryanbraun.com/anchorjs/)
([source](https://github.com/bryanbraun/anchorjs)) | [`4.3.1` ->
`5.0.0`](https://renovatebot.com/diffs/npm/anchor-js/4.3.1/5.0.0) |
[![age](https://badges.renovateapi.com/packages/npm/anchor-js/5.0.0/age-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://badges.renovateapi.com/packages/npm/anchor-js/5.0.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://badges.renovateapi.com/packages/npm/anchor-js/5.0.0/compatibility-slim/4.3.1)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://badges.renovateapi.com/packages/npm/anchor-js/5.0.0/confidence-slim/4.3.1)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>bryanbraun/anchorjs</summary>

###
[`v5.0.0`](https://github.com/bryanbraun/anchorjs/releases/tag/5.0.0)

[Compare
Source](https://github.com/bryanbraun/anchorjs/compare/4.3.1...5.0.0)

##### Breaking Changes:

- The `touch` option on the `visibility` setting has been removed. Now
the only visibility settings are `hover` and `always`. Sites currently
using `touch` (or any other unsupported `visibility` option) will fall
back to the behavior for `hover`. See [this
comment](https://github.com/bryanbraun/anchorjs/issues/188#issuecomment-1382674984)
and [this
commit](https://github.com/bryanbraun/anchorjs/commit/96c2cc30aa9895bebce8def0da856229a11183ad).

##### Features:

- Add support for ESM global imports. See [this
comment](https://github.com/bryanbraun/anchorjs/issues/109#issuecomment-639102418)
and [this
commit](https://github.com/bryanbraun/anchorjs/commit/8ae6db23ef4f8e3e3fde1c2d1be1bbba0d054a40).
- Center the focus indicator for keyboard users:
[https://github.com/bryanbraun/anchorjs/pull/192](https://github.com/bryanbraun/anchorjs/pull/192)

##### Other:

- Updates to devDependencies
([`d0dabc4`](https://github.com/bryanbraun/anchorjs/commit/d0dabc4ba03338aea544991357ccaea9e8a1eee9),
[`d7d212b`](https://github.com/bryanbraun/anchorjs/commit/d7d212b8740b12f27fc74ac2063d843adf0c9ae8),
[`d7d212b`](https://github.com/bryanbraun/anchorjs/commit/d7d212b8740b12f27fc74ac2063d843adf0c9ae8)),
tests
([`fbc80e0`](https://github.com/bryanbraun/anchorjs/commit/fbc80e0ed767df334adaf9c6fb56779790a44fd6)),
and docs
([`30f1b6a`](https://github.com/bryanbraun/anchorjs/commit/30f1b6a0a8b9ae8a1ccb0d3d9db356b100234477),
[`ab52088`](https://github.com/bryanbraun/anchorjs/commit/ab5208854b57e0f4b27f5bb77e4de696503d7d68),
[`2bed64a`](https://github.com/bryanbraun/anchorjs/commit/2bed64a241416fe3523ee7fa2afa25d7bcd3894c)).
-   Switched the primary development branch from `master` → `main`

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://app.renovatebot.com/dashboard#github/jenkins-infra/jenkins.io).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMDguMSIsInVwZGF0ZWRJblZlciI6IjM0LjEwOC4xIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

2 participants