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.

Sign up for GitHub

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

Jump to bottom

Masterbar: Revert recent changes #12454

Merged
merged 3 commits into from
May 23, 2019
Merged

Masterbar: Revert recent changes #12454

merged 3 commits into from
May 23, 2019

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented May 23, 2019

See p9Jlb4-R8-p2

Reverts #12299, #11766.

The masterbar needs to provide a way to log out of the local site.

Changes proposed in this Pull Request:

  • Restore previous masterbar.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

  • p9Jlb4-R8-p2

Testing instructions:

  • With the WordPress.com Toolbar module active, attempt to use the logout button in the masterbar.

Proposed changelog entry for your changes:

  • WordPress.com Toolbar: Restore the previous layout.

@kraftbj kraftbj added [Type] Bug When a feature is broken and / or not performing as intended [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Feature] Masterbar WordPress.com Toolbar and Dashboard customizations [Status] Needs Cherry-Pick labels May 23, 2019
@kraftbj kraftbj requested review from a team as code owners May 23, 2019 16:04
@kraftbj
Copy link
Contributor Author

kraftbj commented May 23, 2019

On the 7.3 branch, need to revert c6bf1c2 then f6e7433 for the cherry-pick I believe.

davemart-in
davemart-in previously approved these changes May 23, 2019
View reviewed changes
Copy link
Contributor

@davemart-in davemart-in left a comment

Choose a reason for hiding this comment

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

I approve of this, but I'm not sure I have the qualifications to review/approve the actual code changes.

@kraftbj
Copy link
Contributor Author

kraftbj commented May 23, 2019

Thanks @davemart-in. That's the approval I was hoping from you -- just making sure you're in the loop and 👍 with it from the Design/Product side of it.

@jetpackbot
Copy link

jetpackbot commented May 23, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: June 4, 2019.
Scheduled code freeze: May 28, 2019

Generated by 🚫 dangerJS against 8ed2d9f

@jeherve jeherve added this to the 7.4 milestone May 23, 2019
@jeherve
Copy link
Member

jeherve commented May 23, 2019

Could not cleanly apply this PR to synced wpcom files.

This is fine. This is because D26195-code was not committed yet.

@jeherve jeherve added [Status] In Progress and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels May 23, 2019
@jeherve jeherve added [Status] Needs Review To request a review from Crew. Label will be renamed soon. and removed [Status] In Progress labels May 23, 2019
@jeherve
Copy link
Member

jeherve commented May 23, 2019

Noting that we'll need to re-open all the issues from #11766 when we merge this.

jeherve
jeherve approved these changes May 23, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This seems to work well for me, on mobile and on desktop.

@jeherve jeherve removed the [Status] Needs Review To request a review from Crew. Label will be renamed soon. label May 23, 2019
@jeherve jeherve mentioned this pull request May 23, 2019
Closed
@kraftbj
Copy link
Contributor Author

kraftbj commented May 23, 2019

Noting that we'll need to re-open all the issues from #11766 when we merge this.

My hope is this is a short-term revert until the next attempt lands but yeah, should re-open them for now.

kraftbj
kraftbj commented May 23, 2019
View reviewed changes
Copy link
Contributor Author

@kraftbj kraftbj left a comment

Choose a reason for hiding this comment

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

This worked for me to log out of the local site. It didn't log me out of wp.com, but it at least restores local log out ability.

@kraftbj kraftbj merged commit 6a53622 into master May 23, 2019
@kraftbj kraftbj deleted the revert/masterbar branch May 23, 2019 20:07
kraftbj added a commit that referenced this pull request May 23, 2019
@kraftbj
Masterbar: Revert recent changes (#12454)
f5c3edc 
* Revert "Masterbar: display a Hamburger menu on mobile (#12299)"

This reverts commit fc9811b.

* Revert of f6e7433

* Fix remaining PHPCS warnings
kraftbj added a commit that referenced this pull request May 23, 2019
@kraftbj
Masterbar: Revert recent changes (#12454)
bd3a46a 
* Revert "Masterbar: display a Hamburger menu on mobile (#12299)"

This reverts commit fc9811b.

* Revert of f6e7433

* Fix remaining PHPCS warnings
This was referenced May 24, 2019
Open
Open
Open
Open
Closed
jeherve added a commit that referenced this pull request May 24, 2019
@jeherve
Changelog: add #12454
18624de
jeherve added a commit that referenced this pull request May 27, 2019
@kraftbj @jeherve
7.4 Changelog (#12304)
0fb4436 
* Kick off the changelog

* Add 7.3.1

* Update date and post link

* changelog: add #12219

* changelog: add #12170

* changelog: add #12184

* Changelog: add #12268

* Changelog: add #12081

* Changelog: add #12323

* Changelog: add #12204

* Changelog: add #12269

* Changelog: add #12332

* changelog: add #12339

* changelog: add #12209

* Changelog: add #12319

* Changelog: add #12357

* Changelog: add #12124

* Changelog: add #12373

* Changelog: add #12252

* Changelog: add #12383

* Changelog: add #12372

* changelog: add #12337

* Changelog: add #12290

* Changelog: add #12301

* Changelog: add #12061

* Testing list: add instructions for #12061

* Changelog: add #12393

* Update minimum supported version

See #12287

* Changelog: add #12406

* Testing list: add #12406

* Changelog: add #12277

* Changelog: add #12412

* Changelog: add #11318

* Changelog: add #12328

* Changelog: add #12425

* Changelog: add #12380

* Changelog: add #12428

* Changelog: add #12414

* Changelog: add #12395

* Changelog & Testing list: add #12416, #12417, #12418, and #12348

* changelog: add #12379

* Changelog: add #12341

* changelog: add #12444

* Changelog: add #12434

* Changelog: add #12454

* Changelog: add #12460

* Changelog: add #12463

* Changelog: add #12457

* Changelog / testing list: add #10333

* Changelog: add #12467


Co-authored-by: Jeremy Herve <jeremy@jeremy.hu>
@jeherve jeherve mentioned this pull request Jun 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeherve jeherve jeherve approved these changes

@davemart-in davemart-in davemart-in left review comments

@jmdodd jmdodd Awaiting requested review from jmdodd

Assignees
No one assigned
Labels
[Feature] Masterbar WordPress.com Toolbar and Dashboard customizations Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.
5 participants
@kraftbj @jetpackbot @jeherve @davemart-in @matticbot