Bug 177130 – [css-grid] Small refactoring adding RenderGrid::contentAlignment()

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Manuel Rego Casasnovas]

[css-grid] Small refactoring adding RenderGrid::contentAlignment()

[Manuel Rego Casasnovas]

Created attachment 321194 [details]
Patch

[Build Bot]

Attachment 321194 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderGrid.cpp:1586:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Manuel Rego Casasnovas]

Created attachment 321195 [details]
Patch

[Javier Fernandez]

Comment on attachment 321195 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321195&action=review

The change looks good to me.

> Source/WebCore/ChangeLog:10
> +        that is from RenderGrid and GridTrackSizingAlgorithm.

I think there is something missing in this sentence; perhaps "accessible" ?

[Manuel Rego Casasnovas]

Comment on attachment 321195 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321195&action=review

Thanks for the review, uploading new version fixing ChangeLog.

>> Source/WebCore/ChangeLog:10
>> +        that is from RenderGrid and GridTrackSizingAlgorithm.
> 
> I think there is something missing in this sentence; perhaps "accessible" ?

Actually I was thinking in "that is used from", but indeed I didn't write it.

[Manuel Rego Casasnovas]

Created attachment 321197 [details]
Patch

[Sergio Villar Senin]

Comment on attachment 321197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321197&action=review

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:1078
> +        || (m_renderGrid->contentAlignment(m_direction).distribution() != ContentDistributionStretch))

Very nice. Do we still need resolvedJustifyContentPosition() and resolvedJustifyContentDistribution() ?

[Javier Fernandez]

Comment on attachment 321197 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=321197&action=review

>> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:1078
>> +        || (m_renderGrid->contentAlignment(m_direction).distribution() != ContentDistributionStretch))
> 
> Very nice. Do we still need resolvedJustifyContentPosition() and resolvedJustifyContentDistribution() ?

Perhaps not, but I think they are both useful functions when only needing specific content alignment values.

[Manuel Rego Casasnovas]

(In reply to Sergio Villar Senin from comment #7)
> Comment on attachment 321197 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321197&action=review
> 
> > Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:1078
> > +        || (m_renderGrid->contentAlignment(m_direction).distribution() != ContentDistributionStretch))
> 
> Very nice. Do we still need resolvedJustifyContentPosition() and
> resolvedJustifyContentDistribution() ?

Some of these are still used by RenderFlexibleBox so I guess
it's not an issue to keep them.

[WebKit Commit Bot]

Comment on attachment 321197 [details]
Patch

Rejecting attachment 321197 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 321197, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
71.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp.rej
patching file Source/WebCore/rendering/RenderGrid.cpp
patching file Source/WebCore/rendering/RenderGrid.h
patching file Source/WebCore/rendering/style/RenderStyle.cpp
patching file Source/WebCore/rendering/style/RenderStyle.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Sergio Villar Senin']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: http://webkit-queues.webkit.org/results/4647172

[Manuel Rego Casasnovas]

Created attachment 321673 [details]
=Patch for landing

[WebKit Commit Bot]

Comment on attachment 321673 [details]
=Patch for landing

Clearing flags on attachment: 321673

Committed r222441: <http://trac.webkit.org/changeset/222441>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34693381>