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

fix scrolling and renaming in large folders #759

Merged
merged 2 commits into from Apr 14, 2017
Merged

fix scrolling and renaming in large folders #759

merged 2 commits into from Apr 14, 2017

Conversation

raveit65
Copy link
Member

fixes #755
First time it's a bit hard to reproduce the issue, see this posts.
#755 (comment)
#755 (comment)
https://www.dropbox.com/s/8ftp9gois751892/caja-issue.ogv?dl=0

Note, i added a cosmetic back port of a nemo commit in another branch (dev-experimental), which reduce the width of the gap between grid of items and vertical right scrollbar.
Taken from linuxmint/nemo#1257
But this dynamically grid change the look and feel of the icon-view and isn't really needed to fix the issue.
Please vote here if you like this behaviour or not.

Copy link
Member

@lukefromdc lukefromdc left a comment

Choose a reason for hiding this comment

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

Works, fixes the problem same as in my earlier test of same code without the cosmetic grid change. Happy with it either way but this keeps the appearance the same and in my judgement looks a bit better

@raveit65
Copy link
Member Author

@sc0w
please test and vote

 relayouting

The GtkScrolledWindow uses the widget prefered size as a guess as to
whether scrollbars are needed or not in the automatic scrollbars case.
If we don't report anything for them we typically get it wrong and cause
two size allocate calls on the child each time, with different sizes.
This (the two sizes speicifically) will cause unnecessary relayouts of
the window.

So, we just report the current size of the layed out icons as the prefered
size. This is somewhat wrong as its depending on previous size_allocation
calls rather than the "ideal" size of the widget, but since the ideal size
is ignored anyway and just used for this it works well.

taken from:
https://git.gnome.org/browse/nautilus/commit/?id=fa6e447
Not queueing resizes may play oddly with the size request caches in
GTK+, resulting in gtk_widget_get_preferred_width/height returning
0 even after the canvas was populated.

https://bugzilla.gnome.org/show_bug.cgi?id=667831

Taken from:
https://git.gnome.org/browse/nautilus/commit/?id=8c77821
@lukefromdc
Copy link
Member

lukefromdc commented Apr 13, 2017

Both versions (dev-view and dev-experimental) rebased after 4dbf114
EDIT: waitiing for more votes before merging

@sc0w
Copy link
Member

sc0w commented Apr 13, 2017

The fix works for me

The changes in the look are minimal, so, I agree with this PR

@raveit65
Copy link
Member Author

raveit65 commented Apr 14, 2017

The changes in the look are minimal, so, I agree with this PR

The vote was more for the other branch (dev-experimental) with 'Expand grid width to canvas'
60eef1e :-)
The PR here is a a bugfix.
Btw. same happens with mate-control-center window.

@lukefromdc lukefromdc merged commit dddec5f into master Apr 14, 2017
@lukefromdc lukefromdc deleted the dev-view branch April 14, 2017 05:32
@raveit65
Copy link
Member Author

raveit65 commented Apr 14, 2017

@monsta @XRevan86 @flexiondotorg
I really would like to hear you opinion for 'Expand grid width to canvas'
60eef1e
from dev-experimental branch?

Well, it change the look and feel if you rezize caja window, but honestly how often you resize the window.
Mostly i use one size which is good for my monitor size and that's it.
Not sure if someone resize a caja window whole the day ;-)
On the other side the grid have always the best view neither which window size you're using.
And i am pretty shure after 3 weeks you have forgotten that we changed behaviour and use a dynamically grid.

Reopen for voting

@raveit65
Copy link
Member Author

@sbalneav what do you think?

@sc0w
Copy link
Member

sc0w commented Apr 14, 2017

@raveit65

The vote was more for the other branch (dev-experimental) with 'Expand grid width to canvas'
60eef1e :-)

ah, ok

without 60eef1e : caja sometimes shows extra spaces between icons and the right scroll bar

with 60eef1e : fixes it, and the final result resizing the sidebar is better, I think

I agree

👍

@monsta
Copy link
Contributor

monsta commented Apr 14, 2017

I don't like the last comment in linuxmint/nemo#1257...

@raveit65
Copy link
Member Author

raveit65 commented Apr 14, 2017

I don't like the last comment in linuxmint/nemo#1257...

@monsta

see @lukefromdc comment at #755 (comment)
Beside that you don't like to read the comment, could you reproduce it ?

@raveit65
Copy link
Member Author

@monsta
I can't reproduce this, this have to be nemo specific.

@lukefromdc
Copy link
Member

https://github.com/monsta , the Nemo issue mentioned was one caused by the dynamic gred resize that we decided not to merge, and I was not able to reproduce it in builds of Caja that included that commit. Certainly we won't have that as Caja sits not, because we've kept the constant-width columns while still fixing the scroll bug on large folders and maximum right side gap.

@lukefromdc
Copy link
Member

lukefromdc commented Apr 14, 2017

At any rate, dev-experimental is just one more commit that can be easily merged if people want to do so-and as I said I was unable to duplicate the label bug reported against Nemo.
EDIT: this is why I made sure to rebase both branches.

@monsta
Copy link
Contributor

monsta commented Apr 15, 2017

I missed these comments at #755 indeed, I wasn't subscribed to that report. I'll check the "experimental" commit.

@raveit65
Copy link
Member Author

raveit65 commented Apr 15, 2017

@monsta
for your info, transifex commit from 1.18.1 wasn't in master, hope this is the only package.....
For this reason i had to revert a new transifex update first :-/

@monsta
Copy link
Contributor

monsta commented Apr 15, 2017

I don't think I pulled anything from Transifex lately...

@monsta
Copy link
Contributor

monsta commented Apr 15, 2017

Hmm, the new behavior from "experimental" commit might be better than the current. It takes some time to get used to it, so if we merge it, it should be only for master branch.
I also tried to reproduce the mentioned bug from Nemo, but couldn't. I only tested with GTK+ 3.18.9 though.

Should we create a new PR for that change?

@raveit65
Copy link
Member Author

Should we create a new PR for that change?

If you want.....
Some ppl did already vote here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gtk+-3: Caja cannot scroll with touchpad or mouse to bottom of a folder
4 participants