After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 675527 - Bug in searchDisplay.js in line 138, for IconGrid never show more than 1 row
Bug in searchDisplay.js in line 138, for IconGrid never show more than 1 row
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.4.x
Other Linux
: Normal enhancement
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-05-05 19:25 UTC by c.weber23
Modified: 2012-07-13 03:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed patch (968 bytes, patch)
2012-05-08 12:30 UTC, c.weber23
needs-work Details | Review
2nd proposal patch (1.43 KB, patch)
2012-05-08 20:06 UTC, c.weber23
accepted-commit_now Details | Review

Description c.weber23 2012-05-05 19:25:22 UTC
(while writing extensions ..) if you create a new IconGrid with

let grid = new IconGrid.IconGrid({ 
  rowLimit: MAX_ROWS, 
  columnLimit: MAX_RESULTS,
  xAlign: St.Align.START });


the IconGrid is properly created, but when populated with results, never more than one row is shown. This is due line 138 in searchDisplay.js where the max row property is hard coded,
changing "MAX_SEARCH_RESULTS_ROWS" to "this._grid._rowLimit"

fixes the problem.
Comment 1 c.weber23 2012-05-08 10:16:29 UTC
Question: Could I fix it my self, and create a pull request,
or do i need to be an approved bugfixer , or does this bug need to be confirmed by s.o. else first ?
Comment 2 Florian Müllner 2012-05-08 10:43:19 UTC
(In reply to comment #1)
> Question: Could I fix it my self, and create a pull request

That would be great - rather than a pull request, you should create a patch with git-format-patch and attach it here though.
Comment 3 c.weber23 2012-05-08 12:30:32 UTC
Created attachment 213663 [details] [review]
proposed patch
Comment 4 Jasper St. Pierre (not reading bugmail) 2012-05-08 19:28:40 UTC
Review of attachment 213663 [details] [review]:

_rowLimit is private, so no. You should modify iconGrid.js to make it public as well.
Comment 5 c.weber23 2012-05-08 20:06:58 UTC
Created attachment 213700 [details] [review]
2nd proposal patch

huh, didn't know js supports the concept of private & public ;) but now read about it, and adjusted the patch.
Comment 6 Jasper St. Pierre (not reading bugmail) 2012-05-08 20:31:38 UTC
(In reply to comment #5)
> huh, didn't know js supports the concept of private & public

It doesn't. It's just a convention we have.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-06-01 10:55:16 UTC
Review of attachment 213700 [details] [review]:

Sure.
Comment 8 Florian Müllner 2012-06-01 11:13:44 UTC
Actually I've been wondering if this is actually a bug, e.g. should the number of rows be a per-provider property (like with this patch), or should rows-per-provider be a property of the search display (like it is now, to give equal space to each provider)?
Comment 9 Jasper St. Pierre (not reading bugmail) 2012-06-02 09:13:30 UTC
(In reply to comment #8)
> Actually I've been wondering if this is actually a bug, e.g. should the number
> of rows be a per-provider property (like with this patch), or should
> rows-per-provider be a property of the search display (like it is now, to give
> equal space to each provider)?

I think if we have a property, we should use it, not just drop it on the ground half the time.


Christian, do you need somebody to push the patch for you?
Comment 10 c.weber23 2012-06-02 09:58:12 UTC
> Christian, do you need somebody to push the patch for you?

Yes, that'd be nice!
Comment 11 Jasper St. Pierre (not reading bugmail) 2012-07-13 03:42:35 UTC
I pushed this a while ago.