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

Image options are not visible in pop up on clicking replace button from Image block #28604

Closed
wants to merge 1 commit into from
Closed

Image options are not visible in pop up on clicking replace button from Image block #28604

wants to merge 1 commit into from

Conversation

speedyprem
Copy link

@speedyprem speedyprem commented Jan 29, 2021 •
edited
Loading

Description

Image options are not visible in pop up on clicking replace button from Image block due to the horizontal scroll.

WordPress Track ticket: https://core.trac.wordpress.org/ticket/52396

How has this been tested?

1.Create new article.
2.Add image block.
3.Add any image inside the image block.
4.Click on replace button from the image block.

Screenshots

image

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Jan 29, 2021
@speedyprem speedyprem mentioned this pull request Feb 1, 2021
Closed
@paaljoachim
Copy link
Contributor

paaljoachim commented Feb 16, 2021 •
edited
Loading

Hi @speedyprem Prem

Can you create an before and after applying the PR visible result to your PR?
thanks!

As this is what I am seeing in WP 5.6.1 when no plugin is activated.

Screen Shot 2021-02-16 at 09 40 05

Related issue: #27186

@paaljoachim
Copy link
Contributor

paaljoachim commented Feb 16, 2021 •
edited
Loading

One more thing.
This is an issue in 5.6 and 5.6.1
But not an issue in 5.7 beta 2. Meaning a fix has been applied to 5.7.
I believe we need to find the fix added to 5.7 and backport it to 5.6.
@youknowriad @tellthemachines @talldan

@speedyprem
Copy link
Author

Here is the after implementation screenshot:

image

@audrasjb
Copy link
Contributor

Indeed, it would be nice to fix it with 5.6.2. Please feel free to get in touch with me or Jake concerning 5.6.2 timeline :-)

@paaljoachim paaljoachim added the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 16, 2021
desrosj added a commit that referenced this pull request Feb 16, 2021
@desrosj
Backport #28604.
32784b6
@desrosj desrosj mentioned this pull request Feb 16, 2021
Closed
desrosj added a commit that referenced this pull request Feb 16, 2021
@desrosj
Revert "Backport #28604."
128d6a0 
This reverts commit 32784b6.
@desrosj
Copy link
Contributor

desrosj commented Feb 17, 2021

I've been looking into this one today and here's what I've found.

This issue appears to be fixed in master or WordPress trunk (5.7), but the commit directly responsible hasn't jumped out just yet. Because that's the case, I think we should leave master alone, change the base of this PR to be wp/5.6 (the branch used for backporting package updates to the 5.6 branch) and make a change we're comfortable with making just for the 5.6 branch.

There are a few other related PRs and issues created, but I think the change in this PR may be the safest to make without affecting other areas.

@desrosj desrosj changed the base branch from master to wp/5.6 February 17, 2021 00:11
@desrosj desrosj changed the base branch from wp/5.6 to master February 17, 2021 00:12
desrosj added a commit that referenced this pull request Feb 17, 2021
@desrosj
Fix bug where options are not visible when clicking replace image.
62013ab 
This is a more specific approach to fixing #28604, targeting only the popover in question.
@desrosj desrosj mentioned this pull request Feb 17, 2021
Closed
7 tasks
@desrosj
Copy link
Contributor

desrosj commented Feb 17, 2021

After working through this with @tellthemachines, I've created #29057 to replace this one. It branches off of wp/5.6 instead of master (switching bases and rebasing did not work here), and takes a more targeted approach of fixing only the specific popover affected to prevent any unintended changes elsewhere.

@desrosj desrosj closed this Feb 17, 2021
@tellthemachines tellthemachines removed the Backport to WP Minor Release Pull request that needs to be backported to a WordPress minor release label Feb 17, 2021
tellthemachines added a commit that referenced this pull request Feb 17, 2021
@tellthemachines @desrosj
@kevin940726 @talldan @aaronrobertshaw
WP 5.6.2 backports (#29058)
0886f88 
* Backport #26475 for the 5.6 branch.

* Backport #28604.

* Revert "Backport #28604."

This reverts commit 32784b6.

* Backport #26583 to the 5.6 branch.

* Commit lock file updates.

* Committing lock file differences after updating `browserslist`.

* Pin the version of NodeJS in the Compressed Size workflow.

* Memoize getEntityRecords to prevent infinite re-renders (#26447)

* Fix issue where gallery block requests all attachments when empty (#28621)

* Return early from building attachments for galleries

* Improve code clarity

* Fix link control styles to prevent scrollbar (#27777)

* Update package-lock

* Update package-lock again

Co-authored-by: Jonathan Desrosiers <desrosj@users.noreply.github.com>
Co-authored-by: Kai Hao <kevin830726@gmail.com>
Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
@ash-f ash-f mentioned this pull request Feb 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajitbohra ajitbohra Awaiting requested review from ajitbohra

@chrisvanpatten chrisvanpatten Awaiting requested review from chrisvanpatten

@ajlende ajlende Awaiting requested review from ajlende

@atimmer atimmer Awaiting requested review from atimmer

@cameronvoell cameronvoell Awaiting requested review from cameronvoell

@dmsnell dmsnell Awaiting requested review from dmsnell

@ellatrix ellatrix Awaiting requested review from ellatrix

@etoledom etoledom Awaiting requested review from etoledom

@guarani guarani Awaiting requested review from guarani

@gziolo gziolo Awaiting requested review from gziolo

@karmatosed karmatosed Awaiting requested review from karmatosed

@mapk mapk Awaiting requested review from mapk

@mkaz mkaz Awaiting requested review from mkaz

@mkevins mkevins Awaiting requested review from mkevins

@mmtr mmtr Awaiting requested review from mmtr

@nerrad nerrad Awaiting requested review from nerrad

@oandregal oandregal Awaiting requested review from oandregal

@ntwb ntwb Awaiting requested review from ntwb

@Soean Soean Awaiting requested review from Soean

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey

@talldan talldan Awaiting requested review from talldan

@TimothyBJacobs TimothyBJacobs Awaiting requested review from TimothyBJacobs

@youknowriad youknowriad Awaiting requested review from youknowriad

Assignees
No one assigned
Labels
First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@speedyprem @paaljoachim @audrasjb @desrosj @tellthemachines