fix(docs-infra): improve focus styles in topnav and footer by sjtrimble · Pull Request #33255 · angular/angular

sjtrimble · 2019-10-18T17:54:32Z

Fixes #33239

Improved focus styles for topnav and footer including visible color.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Topnav:

Footer:

Issue Number: #33239

What is the new behavior?

Topnav:

Footer:

Does this PR introduce a breaking change?

Yes
No

mary-poppins · 2019-10-18T21:35:08Z

You can preview 735eefd at https://pr33255-735eefd.ngbuilds.io/.

petebacondarwin

Hi @sjtrimble - good to see you back to work.
When I look at the last preview URL, it doesn't look the same as your "new behaviour" animations.
Are they out of date?

Apart from that the focussing looks much better than before.

Also, perhaps out of range for this PR, but it is weird that if the browser is the same width as in your example animations, focussing the search box actually make it go smaller. Can we fix that?

sjtrimble · 2019-10-21T20:02:53Z

@petebacondarwin Heya! Weird, to me the preview URL and the GIF look the same. What is different?

That is super weird about the search box! I'll open an issue for it. It seems to happen in a smaller screen size.

sjtrimble · 2019-10-21T20:03:48Z

@gkalpak here's the PR that needs your help with getting active state/class on the topnav links for styling.

gkalpak · 2019-10-22T09:48:52Z

@sjtrimble, let's fix the focus styles on this PR and I'll create a separate one for highlighting active tobbar nav links. EDIT: Created #33351.

As Pete mentioned, the styling on the preview looks different than your gifs 😁
Here is what it looks like on my laptop:

The main (undesirable) differences I see are:

There is no space between the logo and the focus outline.
The topbar nav links still have the lighter background color (in addition to the outline) when focused.
There is too much space between the Twitter and GitHub logos and their focus outline (and each outline intersects with the other logo).

Does it look different on your machine?

sjtrimble · 2019-10-22T18:12:39Z

@gkalpak Super strange .. yeah, it doesn't look like that on my machine. I'm using a Chrome browser on my Mac both on my local version and in the PR preview link 🤔

I checked on Safari and it changes the color there for some of the outlines which is also strange ..

sonukapoor · 2020-01-03T01:01:11Z

@gkalpak I have a fix for this ready. I will discuss it with you offline.

sonukapoor · 2020-01-03T14:56:23Z

@sjtrimble / @gkalpak Please see followup PR with a fix:

#34634

gkalpak · 2020-01-17T12:26:59Z

@sjtrimble, could you take a look at my fixup commit and verify you are happy with the changes and they look OK on macOS?

gkalpak · 2020-01-17T16:36:07Z

I pushed another fixup commit to fix CI.

mary-poppins · 2020-01-17T16:44:16Z

You can preview 9073352 at https://pr33255-9073352.ngbuilds.io/.

sjtrimble · 2020-02-18T23:01:31Z

Need to add outline-width: 1px so it doesn't look excessive.

sjtrimble · 2020-02-18T23:02:05Z

Need to add outline-width: 1px so it doesn't look excessive.

Done (and also for the social icons).

gkalpak · 2020-02-19T11:09:47Z

Rebased on master and explicitly set the outline-width to 1px. Thx, @sjtrimble!
I am marking this for merging. If there are more tweaks needed, we can make them in a follow-up PR.

mary-poppins · 2020-02-19T11:14:35Z

You can preview 02ffecd at https://pr33255-02ffecd.ngbuilds.io/.

gkalpak · 2020-02-19T11:34:32Z

(The payload size failure in test_aio_local will be fixed with #35538.)

Fixes angular#33239

mary-poppins · 2020-02-19T17:42:04Z

You can preview a088d00 at https://pr33255-a088d00.ngbuilds.io/.

Fixes #33239 PR Close #33255

Related to angular#33239 and angular#33255.

Related to #33239 and #33255. PR Close #33351

angular-automatic-lock-bot · 2020-03-21T16:07:02Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

sjtrimble requested a review from a team October 18, 2019 17:54

googlebot added the cla: yes label Oct 18, 2019

sjtrimble requested a review from gkalpak October 18, 2019 18:03

sjtrimble self-assigned this Oct 18, 2019

sjtrimble added action: review The PR is still awaiting reviews from at least one requested reviewer comp: docs-infra labels Oct 18, 2019

ngbot Bot added this to the needsTriage milestone Oct 18, 2019

petebacondarwin approved these changes Oct 20, 2019

View reviewed changes

gkalpak mentioned this pull request Oct 23, 2019

fix(docs-infra): use .tooltip and highlight the currently active node in top-bar #33351

Closed

gkalpak mentioned this pull request Nov 25, 2019

fix(docs-infra): do not break when cookies are disabled in the browser #33829

Closed

14 tasks

gkalpak added state: WIP target: patch This PR is targeted for the next patch release type: bug/fix and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 10, 2019

gkalpak modified the milestones: needsTriage, docs-infra-aio-app Dec 10, 2019

sonukapoor mentioned this pull request Jan 3, 2020

Fix nav focus active styles #34634

Closed

14 tasks

gkalpak force-pushed the fix-nav-focus-active-styles branch from 735eefd to 7b6fc99 Compare January 17, 2020 12:07

This comment has been minimized.

Sign in to view

googlebot added cla: no and removed cla: yes labels Jan 17, 2020

gkalpak removed the state: WIP label Jan 17, 2020

This comment has been minimized.

Sign in to view

googlebot added cla: yes and removed cla: no labels Jan 17, 2020

This comment has been minimized.

Sign in to view

sjtrimble commented Feb 18, 2020

View reviewed changes

gkalpak force-pushed the fix-nav-focus-active-styles branch from 9073352 to 02ffecd Compare February 19, 2020 11:05

gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 19, 2020

sjtrimble and others added 3 commits February 19, 2020 19:33

fix(docs-infra): improve focus styles in topnav and footer

8a6280d

Fixes angular#33239

fixup! fix(docs-infra): improve focus styles in topnav and footer

f4938f9

fixup! fix(docs-infra): improve focus styles in topnav and footer

a088d00

gkalpak force-pushed the fix-nav-focus-active-styles branch from 02ffecd to a088d00 Compare February 19, 2020 17:33

alxhub pushed a commit that referenced this pull request Feb 19, 2020

fix(docs-infra): improve focus styles in topnav and footer (#33255)

3a97972

Fixes #33239 PR Close #33255

alxhub closed this in 1997b86 Feb 19, 2020

gkalpak added a commit to gkalpak/angular that referenced this pull request Feb 20, 2020

fix(docs-infra): highlight the currently active node in top-bar

0ec3ce9

Related to angular#33239 and angular#33255.

mhevery pushed a commit that referenced this pull request Feb 20, 2020

fix(docs-infra): highlight the currently active node in top-bar (#33351)

7dbbe24

Related to #33239 and #33255. PR Close #33351

mhevery pushed a commit that referenced this pull request Feb 20, 2020

fix(docs-infra): highlight the currently active node in top-bar (#33351)

2aa940f

Related to #33239 and #33255. PR Close #33351

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Mar 21, 2020

Conversation

sjtrimble commented Oct 18, 2019

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Uh oh!

mary-poppins commented Oct 18, 2019

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

sjtrimble commented Oct 21, 2019

Uh oh!

sjtrimble commented Oct 21, 2019

Uh oh!

gkalpak commented Oct 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjtrimble commented Oct 22, 2019

Uh oh!

sonukapoor commented Jan 3, 2020

Uh oh!

sonukapoor commented Jan 3, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

gkalpak commented Jan 17, 2020

Uh oh!

gkalpak commented Jan 17, 2020

Uh oh!

mary-poppins commented Jan 17, 2020

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

sjtrimble Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

gkalpak Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

sjtrimble Feb 18, 2020

Choose a reason for hiding this comment

Uh oh!

gkalpak Feb 19, 2020

Choose a reason for hiding this comment

Uh oh!

gkalpak commented Feb 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mary-poppins commented Feb 19, 2020

Uh oh!

gkalpak commented Feb 19, 2020

Uh oh!

mary-poppins commented Feb 19, 2020

Uh oh!

angular-automatic-lock-bot Bot commented Mar 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

gkalpak commented Oct 22, 2019 •

edited

Loading

gkalpak commented Feb 19, 2020 •

edited

Loading