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

fix(docs-infra): preserves query and hash when switching angular versions #35318

Closed
wants to merge 4 commits into from

Conversation

sonukapoor
Copy link
Contributor

Previously, when switching angular versions through the
version selector in the sidenav, the path is lost, and the
user has to manually navigate to the original location.

This commit fixes this issue and maintains the pathname
when navigating between different versions.

Closes #24495

PR Checklist

Please check if your PR fulfills the following requirements:

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?

Issue Number: #24495

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@sonukapoor
Copy link
Contributor Author

@petebacondarwin / @gkalpak Are we OK with this one?

@gkalpak gkalpak added aio: preview action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release type: bug/fix labels Feb 14, 2020
@mary-poppins
Copy link

You can preview 607b1f6 at https://pr35318-607b1f6.ngbuilds.io/.
You can preview 4cc785e at https://pr35318-4cc785e.ngbuilds.io/.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Thx, @sonukapoor!
Please change the commit message to something like:

fix(docs-infra): maintain pathname ...

aio/src/app/app.component.ts Outdated Show resolved Hide resolved
aio/src/app/app.component.spec.ts Show resolved Hide resolved
@mary-poppins
Copy link

You can preview bf3594e at https://pr35318-bf3594e.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 8ab07f5 at https://pr35318-8ab07f5.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 73836ef at https://pr35318-73836ef.ngbuilds.io/.

@sonukapoor sonukapoor force-pushed the maintain-pathname branch 4 times, most recently from ffe458c to f106d1c Compare February 16, 2020 03:14
@mary-poppins
Copy link

You can preview f106d1c at https://pr35318-f106d1c.ngbuilds.io/.

@mary-poppins
Copy link

You can preview f3cdc12 at https://pr35318-f3cdc12.ngbuilds.io/.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Also, please, make sure you follow the commit message guidelines in all commit messages.

aio/src/app/app.component.ts Outdated Show resolved Hide resolved
aio/src/app/app.component.ts Outdated Show resolved Hide resolved
@sonukapoor
Copy link
Contributor Author

Also, please, make sure you follow the commit message guidelines in all commit messages.

Is there something wrong with the current commit messages of this PR?

@mary-poppins
Copy link

You can preview b9bdeb6 at https://pr35318-b9bdeb6.ngbuilds.io/.

Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

Regarding the commit messages:

  1. The first commit message still mentions only pathname, while the commit now also preserves query and hash.
  2. The subject should not start with a capital letter (i.e. fix(docs-infra): Maintain --> fix(docs-infra): maintain).
  3. The second commit (and its fixup) should use the style type:

    style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)

aio/src/app/app.component.spec.ts Outdated Show resolved Hide resolved
aio/src/app/app.component.ts Outdated Show resolved Hide resolved
…ions

Previously, when switching angular versions through the
version selector in the sidenav, the query and hash is lost.
The user has to manually navigate to the original location again.

This commit fixes this issue and preserves the query and hash
when navigating between different versions.

Closes angular#24495
@mary-poppins
Copy link

You can preview d4e9175 at https://pr35318-d4e9175.ngbuilds.io/.

@sonukapoor sonukapoor changed the title docs: Maintain pathname when switching angular versions fix(docs-infra): preserves query and hash when switching angular versions Feb 17, 2020
@gkalpak gkalpak removed the request for review from petebacondarwin February 18, 2020 10:55
alxhub pushed a commit that referenced this pull request Feb 18, 2020
…ions (#35318)

Previously, when switching angular versions through the
version selector in the sidenav, the query and hash is lost.
The user has to manually navigate to the original location again.

This commit fixes this issue and preserves the query and hash
when navigating between different versions.

Closes #24495

PR Close #35318
alxhub pushed a commit that referenced this pull request Feb 18, 2020
@alxhub alxhub closed this in a788d58 Feb 18, 2020
alxhub pushed a commit that referenced this pull request Feb 18, 2020
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker aio: preview cla: yes target: patch This PR is targeted for the next patch release type: bug/fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

when possible open the same page when user switches to different version
5 participants