docs(docs-infra): move API members cards to the center of the page #58610

hawkgs · 2024-11-12T11:03:13Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/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.dev application / infrastructure changes
Other... Please describe:

What is the current behavior?

The API members cards are rendered on the right-hand side of the main page content.

What is the new behavior?

The members cards are moved below the header, i.e. they are now part of the main page content in the center. This should improve the UX by effectively using the available free space on the page and emphasizing the importance of the members descriptions.

Does this PR introduce a breaking change?

Yes
No

github-actions · 2024-11-12T13:01:46Z

Deployed adev-preview for 5ee82fd to: https://ng-dev-previews-fw--pr-angular-angular-58610-adev-prev-undk8pcw.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

hawkgs · 2024-11-12T15:05:58Z

I'll revert the PR to a draft since I noticed that I've broken the CLI reference details until I issue a fix.

thesmiler · 2024-11-18T13:01:53Z

Every page seems now to add a "Jump to details" at the bottom, which is not a link. Not sure where this is coming from

hawkgs · 2024-11-18T16:03:50Z

Got rid of it.

JeanMeche

We probably should remove the click listener on the cards.
It us useless with the new layout

hawkgs · 2024-11-25T07:59:37Z

Removed. The cards are still focusable but they no longer have listeners. Clicking on a code line and auto-scrolling to its card is still there.

Ahmed-Hakeem · 2024-11-25T18:30:26Z

Every page seems now to add a "Jump to details" at the bottom, which is not a link. Not sure where this is coming from

The problem is that this item and other items with class="cdk-visually-hidden" should be hidden.... we shouldn't remove these tags.

This was fixed by #58812

hawkgs · 2024-11-25T21:10:12Z

I guess we need it for screen readers? I overlooked that. Will revert the change. Strangely, I was not able to reproduce the issue on main last week.

Ahmed-Hakeem · 2024-11-26T15:51:06Z

I guess we need it for screen readers? I overlooked that. Will revert the change. Strangely, I was not able to reproduce the issue on main last week.

Exactly, Elements with these classes are intended for screen readers

hawkgs · 2024-11-27T12:59:43Z

This PR is ready for review.

JeanMeche · 2024-11-29T17:43:56Z

The scrolling looks broken.

ex: https://ng-dev-previews-fw--pr-angular-angular-58610-adev-prev-undk8pcw.web.app/api/router/Router#events

If you click on events, it scrolls down to the bottom of the page.

Ahmed-Hakeem · 2024-11-30T23:27:31Z

If it's final that you will change the cards to be in the bottom permanently ... I believe we will have the opportunity to remove all the resize observer stuff and start using the straight forward scrollIntoView way

It will add extra effort to change though but will save too much operations and we will get better performance

What do you think?

hawkgs · 2024-12-02T08:57:40Z

The scrolling looks broken.

I am unable to reproduce it. Tried that on Chrome, Safari and Firefox. However, it seems that on my local setup I've closed the small "The state of JS survey" banner which is indeed covering the header section of the card when I tested the app from the demo link where the banner hasn't been closed yet:

Maybe this is what you are experiencing? I can add some margin so the scroll stops above the target element.

If it's final that you will change the cards to be in the bottom permanently ... I believe we will have the opportunity to remove all the resize observer stuff and start using the straight forward scrollIntoView way

It will add extra effort to change though but will save too much operations and we will get better performance

I initially planned this as "as small as possible change". I agree that upon further look there is some redundant code which is no longer used and can be removed. And this is actually done in the upcoming PR which is an evolution of this one where the tabs are removed, the template of the API details page is pretty much reduced to a single docs-viewer instance and the respective scrolling services are cleaned from all of the redundant code. It is ready but it's blocked by this one.

JeanMeche · 2024-12-02T13:39:23Z

You're right the demo doesn't exihibit the issue anymore. Maybe I was hitting the cache.

hawkgs · 2024-12-02T13:43:17Z

@JeanMeche, I updated the scroll margin top functionality to include 100px constant margin. Hopefully, this should take into account any banners that we have at the top of the page.

@Ahmed-Hakeem, given the margin top was related in some way to the ResizeObserver I dropped the redundant code from the service.

The change can be inspected in the last commit. I intentionally didn't squash it. Let me know, if there is anything else. If the PR is good to go, I will proceed with the squashing.

ngbot · 2024-12-04T11:48:19Z

I see that you just added the action: merge label, but the following checks are still failing:
     status "pullapprove" is failing
     missing required labels: target: *
     4 pending code reviews

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

…the page Move the members description section below the main content of the page since it is part of the core information about a doc item. The CLI reference details page is affected as well.

pkozlowski-opensource · 2024-12-04T14:26:19Z

This PR was merged into the repository by commit 922ffdb.

The changes were merged into the following branches: main, 19.0.x

…the page (#58610) Move the members description section below the main content of the page since it is part of the core information about a doc item. The CLI reference details page is affected as well. PR Close #58610

angular-automatic-lock-bot · 2025-01-04T00:11:11Z

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.}

…the page (angular#58610) Move the members description section below the main content of the page since it is part of the core information about a doc item. The CLI reference details page is affected as well. PR Close angular#58610

pullapprove bot requested a review from thePunderWoman November 12, 2024 11:03

angular-robot bot added detected: feature PR contains a feature commit area: docs-infra Angular.dev application and infrastructure labels Nov 12, 2024

ngbot bot added this to the Backlog milestone Nov 12, 2024

JeanMeche added the adev: preview label Nov 12, 2024

hawkgs force-pushed the docs/api-reference-details-move-params branch from 067c28a to e30de86 Compare November 12, 2024 14:00

hawkgs marked this pull request as draft November 12, 2024 15:06

hawkgs force-pushed the docs/api-reference-details-move-params branch from 149838e to 41b3257 Compare November 13, 2024 12:34

hawkgs changed the title ~~feat(docs-infra): move API members cards to the center of the page~~ docs(docs-infra): move API members cards to the center of the page Nov 14, 2024

hawkgs force-pushed the docs/api-reference-details-move-params branch 2 times, most recently from 4999cc8 to 1e4c039 Compare November 18, 2024 10:43

hawkgs marked this pull request as ready for review November 18, 2024 10:44

thesmiler requested a review from jelbourn November 18, 2024 12:59

hawkgs force-pushed the docs/api-reference-details-move-params branch from 1e4c039 to e5fbe63 Compare November 18, 2024 16:01

JeanMeche requested changes Nov 22, 2024

View reviewed changes

hawkgs force-pushed the docs/api-reference-details-move-params branch from e5fbe63 to a850dfa Compare November 25, 2024 07:55

thesmiler requested a review from JeanMeche November 25, 2024 10:02

hawkgs force-pushed the docs/api-reference-details-move-params branch from a850dfa to f7c2e7b Compare November 26, 2024 08:42

JeanMeche mentioned this pull request Dec 1, 2024

docs(docs-infra): handle the long API names in the API cards #58986

Closed

9 tasks

hawkgs force-pushed the docs/api-reference-details-move-params branch from 2492758 to 4c48f66 Compare December 2, 2024 13:39

hawkgs force-pushed the docs/api-reference-details-move-params branch from 4c48f66 to 1cf9ddf Compare December 2, 2024 13:44

mgechev approved these changes Dec 4, 2024

View reviewed changes

mgechev added the action: merge The PR is ready for merge by the caretaker label Dec 4, 2024

mgechev added the target: patch This PR is targeted for the next patch release label Dec 4, 2024

JeanMeche approved these changes Dec 4, 2024

View reviewed changes

JeanMeche removed request for jelbourn and thePunderWoman December 4, 2024 11:54

feat(docs-infra): move members descriptions section to the center of …

5ee82fd

…the page Move the members description section below the main content of the page since it is part of the core information about a doc item. The CLI reference details page is affected as well.

hawkgs force-pushed the docs/api-reference-details-move-params branch from 1cf9ddf to 5ee82fd Compare December 4, 2024 11:59

Ahmed-Hakeem mentioned this pull request Dec 4, 2024

docs(docs-infra): fix the global layout of the site #58831

Closed

11 tasks

pkozlowski-opensource closed this in 922ffdb Dec 4, 2024

hawkgs deleted the docs/api-reference-details-move-params branch December 4, 2024 14:49

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 4, 2025

docs(docs-infra): move API members cards to the center of the page #58610

docs(docs-infra): move API members cards to the center of the page #58610

Uh oh!

Conversation

hawkgs commented Nov 12, 2024

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Uh oh!

github-actions bot commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hawkgs commented Nov 12, 2024

Uh oh!

thesmiler commented Nov 18, 2024

Uh oh!

hawkgs commented Nov 18, 2024

Uh oh!

JeanMeche left a comment

Choose a reason for hiding this comment

Uh oh!

hawkgs commented Nov 25, 2024

Uh oh!

Ahmed-Hakeem commented Nov 25, 2024

Uh oh!

hawkgs commented Nov 25, 2024

Uh oh!

Ahmed-Hakeem commented Nov 26, 2024

Uh oh!

hawkgs commented Nov 27, 2024

Uh oh!

JeanMeche commented Nov 29, 2024

Uh oh!

Ahmed-Hakeem commented Nov 30, 2024

Uh oh!

hawkgs commented Dec 2, 2024

Uh oh!

JeanMeche commented Dec 2, 2024

Uh oh!

hawkgs commented Dec 2, 2024

Uh oh!

ngbot bot commented Dec 4, 2024

Uh oh!

pkozlowski-opensource commented Dec 4, 2024

Uh oh!

angular-automatic-lock-bot bot commented Jan 4, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Nov 12, 2024 •

edited

Loading