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

Make sure attachments window fills entire sidebar #476

Merged
merged 2 commits into from
Jul 27, 2020

Conversation

sbalneav
Copy link
Contributor

When looking at a document that has attachments, the attachment window only opens up a very small portion of the sidebar up at the top. Looking at the source code for another document viewer from another project cough evince cough I found you needed to have a call to gtk_box_pack_start () in order to have it fill out the sidebar. Here's a pull request that fixes the issue. Tested on Debian 10, works.

@raveit65
Copy link
Member

Hi, long time ago that i've seen you :)
I can confirm the issue.

@@ -565,6 +565,7 @@ ev_sidebar_attachments_init (EvSidebarAttachments *ev_attachbar)
gtk_container_add (GTK_CONTAINER (swindow),
ev_attachbar->priv->icon_view);

gtk_box_pack_start (GTK_BOX (ev_attachbar), swindow, TRUE, TRUE, 0);
Copy link
Member

Choose a reason for hiding this comment

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

Can you fix the indent please?
Sadly, the code uses tabs here (not our code-style), but you should do the same for one new line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, updated my tree, changed the spaces to tabs!

@raveit65
Copy link
Member

raveit65 commented Jul 26, 2020

Attachments sidebar looks much better now. So only code-formatting needs to be fixed.

@raveit65 raveit65 self-requested a review July 27, 2020 19:59
Copy link
Member

@raveit65 raveit65 left a comment

Choose a reason for hiding this comment

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

LGTM

@raveit65 raveit65 merged commit 8e981c6 into mate-desktop:master Jul 27, 2020
@rbuj
Copy link
Contributor

rbuj commented Aug 2, 2020

A new warning message floods the system logs after merging this commit:

(atril:2052): Gtk-WARNING **: 18:50:07.353: Attempting to add a widget with type GtkScrolledWindow to a container of type EvSidebarAttachments, but the widget is already inside a container of type EvSidebarAttachments, please remove the widget from its existing container first.

@raveit65
Copy link
Member

raveit65 commented Aug 2, 2020

Confirmed, should i revert the commit?
I thought this was better tested by @sbalneav (old team member) :/

@raveit65
Copy link
Member

@sbalneav @rbuj
Any ideas to fix the warning or should revert the commit?

@rbuj
Copy link
Contributor

rbuj commented Aug 24, 2020

if gtk_box_pack_start is used then gtk_container_add can be removed.

diff --git a/shell/ev-sidebar-attachments.c b/shell/ev-sidebar-attachments.c
index 389d169..2bcaedb 100644
--- a/shell/ev-sidebar-attachments.c
+++ b/shell/ev-sidebar-attachments.c
@@ -566,8 +566,10 @@ ev_sidebar_attachments_init (EvSidebarAttachments *ev_attachbar)
                           ev_attachbar->priv->icon_view);
 
        gtk_box_pack_start (GTK_BOX (ev_attachbar), swindow, TRUE, TRUE, 0);
+/*
        gtk_container_add (GTK_CONTAINER (ev_attachbar),
                           swindow);
+*/
        gtk_widget_show_all (GTK_WIDGET (ev_attachbar));
 
        /* Icon Theme */

test

$ pdftk doc.pdf attach_file hello.txt output doc.page_attachment.pdf
$ for i in {1..100}; do cp hello.txt hello$i.txt; pdftk doc.page_attachment.pdf attach_file hello$i.txt output doc.page_attachment.2.pdf; rm -f hello$i.txt;mv doc.page_attachment.2.pdf doc.page_attachment.pdf; done

raveit65 added a commit that referenced this pull request Aug 26, 2020
Fixes a runtime warning caused by
70f42da
See #476 (comment)
for more info.
@raveit65
Copy link
Member

PR #487

raveit65 added a commit that referenced this pull request Aug 26, 2020
Fixes a runtime warning caused by
70f42da
See #476 (comment)
for more info.
rbuj pushed a commit that referenced this pull request Aug 28, 2020
Fixes a runtime warning caused by
70f42da
See #476 (comment)
for more info.
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.

None yet

3 participants