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

Gallery Block: show InspectorControls (1 image) #3270

Merged
merged 2 commits into from
Nov 3, 2017
Merged

Gallery Block: show InspectorControls (1 image) #3270

merged 2 commits into from
Nov 3, 2017

Conversation

Soean
Copy link
Member

@Soean Soean commented Oct 31, 2017

Description

The gallery settings should also be visible if the gallery has just 1 image. There is already a check for 0 images (line 161), so we can completely remove the length-check.

How Has This Been Tested?

  1. Add a Gallery Block
  2. Open the block settings
  3. The gallery settings should be visible.

@aduth
Copy link
Member

aduth commented Nov 1, 2017

Is it clear why this condition exists in the first place? Perhaps that it's odd to see the Columns control when only one image exists? Should we consider hiding that specific control when only one image exists in the set?

@aduth aduth added the [Feature] Blocks Overall functionality of blocks label Nov 1, 2017
@codecov
Copy link

codecov bot commented Nov 1, 2017

Codecov Report

Merging #3270 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3270      +/-   ##
==========================================
+ Coverage   31.02%   31.05%   +0.03%     
==========================================
  Files         232      232              
  Lines        6518     6521       +3     
  Branches     1163     1163              
==========================================
+ Hits         2022     2025       +3     
  Misses       3770     3770              
  Partials      726      726
Impacted Files Coverage Δ
blocks/library/gallery/block.js 10% <ø> (ø) ⬆️
editor/header/preview-button/index.js 95.83% <0%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce7870c...237445b. Read the comment docs.

@Soean
Copy link
Member Author

Soean commented Nov 1, 2017

@aduth You are right, the condition was introduced in #1135 for the column slider:
c2732e7?diff=unified#diff-a7fc58c9ade4d7f0d4de7abd9f6275aaR150

A slider with min 1 and max 1 doesn't make sense. So I made an additional commit to add the conditional check just for the slider.

@youknowriad youknowriad merged commit 4da6c0f into WordPress:master Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants