Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/ --- (Updated July 15, 2017, 12:53 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit 0296b89714ce96b247322d9be09b387573d79193 by David Faure on behalf of Simone Gaiarin to branch master. Bugs: 359233 https://bugs.kde.org/show_bug.cgi?id=359233 Repository: kio Description --- The current "open with" dialog implementation does not follow the KDE principle "Simple by default, powerful when needed" for the following reasons: - The "run in terminal" and "keep terminal open" options are advanced options and should not be exposed by default - The primary goal of the dialog should be to select an application from the app tree, running command is an advanced feature My patch changes the behavior as follow: - Put the two options in a KCollapsibleComboBox collapsed by default - The user can expand it only if he needs to use a command line command Implementation details: - When the KCollapsibleComboBox is clicked it is expanded upward keeping the dialog size fixed and compressing the treeview, I'm not sure this is the best approach, but to make it expand downwards we need to fix the size of the dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be desiderable. Maybe there is a way to keep the dialog resizable and expand the combobox downwards, but I couldn't find it. - I've increased the vertical size of the dialog (which I think it was too small) also to accomodate the upward expansion which otherwise would make the app tree almost disappear Relevant discussions: https://bugs.kde.org/show_bug.cgi?id=359233 https://forum.kde.org/viewtopic.php?f=285=131014 Diffs - src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 Diff: https://git.reviewboard.kde.org/r/130180/diff/ Testing --- Thanks, Simone Gaiarin
Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
> On July 15, 2017, 9:31 a.m., Simone Gaiarin wrote: > > Inviala! > > Luigi Toscano wrote: > You can't "ship it" you own review. "Ship it" is marked by other > reviewers who thinks that this should go in. > Do you mean to say that you don't have a developer account and someone > else should push this? > > Simone Gaiarin wrote: > I'm not very familiar with review board. Indeed I don't have a developer > account. I thought that once approved by clicking ship it it would go > through, but then I realized what "ship it" is for, so I was just waiting for > someone to push it. So yes, I need someone with a developer account to push > it please. > > David Faure wrote: > I can take care of it, but I need your email address for the git > authorship information. See linked bug report. - Christoph --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/#review103453 --- On July 12, 2017, 8:54 p.m., Simone Gaiarin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130180/ > --- > > (Updated July 12, 2017, 8:54 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 359233 > https://bugs.kde.org/show_bug.cgi?id=359233 > > > Repository: kio > > > Description > --- > > The current "open with" dialog implementation does not follow the KDE > principle "Simple by default, powerful when needed" for the following reasons: > - The "run in terminal" and "keep terminal open" options are advanced options > and should not be exposed by default > - The primary goal of the dialog should be to select an application from the > app tree, running command is an advanced feature > > My patch changes the behavior as follow: > > - Put the two options in a KCollapsibleComboBox collapsed by default > - The user can expand it only if he needs to use a command line command > > > Implementation details: > - When the KCollapsibleComboBox is clicked it is expanded upward keeping the > dialog size fixed and compressing the treeview, I'm not sure this is the best > approach, but to make it expand downwards we need to fix the size of the > dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be > desiderable. Maybe there is a way to keep the dialog resizable and expand the > combobox downwards, but I couldn't find it. > - I've increased the vertical size of the dialog (which I think it was too > small) also to accomodate the upward expansion which otherwise would make the > app tree almost disappear > > > Relevant discussions: > https://bugs.kde.org/show_bug.cgi?id=359233 > https://forum.kde.org/viewtopic.php?f=285=131014 > > > Diffs > - > > src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 > > Diff: https://git.reviewboard.kde.org/r/130180/diff/ > > > Testing > --- > > > Thanks, > > Simone Gaiarin > >
Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
> On July 15, 2017, 7:31 a.m., Simone Gaiarin wrote: > > Inviala! > > Luigi Toscano wrote: > You can't "ship it" you own review. "Ship it" is marked by other > reviewers who thinks that this should go in. > Do you mean to say that you don't have a developer account and someone > else should push this? > > Simone Gaiarin wrote: > I'm not very familiar with review board. Indeed I don't have a developer > account. I thought that once approved by clicking ship it it would go > through, but then I realized what "ship it" is for, so I was just waiting for > someone to push it. So yes, I need someone with a developer account to push > it please. I can take care of it, but I need your email address for the git authorship information. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/#review103453 --- On July 12, 2017, 6:54 p.m., Simone Gaiarin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130180/ > --- > > (Updated July 12, 2017, 6:54 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 359233 > https://bugs.kde.org/show_bug.cgi?id=359233 > > > Repository: kio > > > Description > --- > > The current "open with" dialog implementation does not follow the KDE > principle "Simple by default, powerful when needed" for the following reasons: > - The "run in terminal" and "keep terminal open" options are advanced options > and should not be exposed by default > - The primary goal of the dialog should be to select an application from the > app tree, running command is an advanced feature > > My patch changes the behavior as follow: > > - Put the two options in a KCollapsibleComboBox collapsed by default > - The user can expand it only if he needs to use a command line command > > > Implementation details: > - When the KCollapsibleComboBox is clicked it is expanded upward keeping the > dialog size fixed and compressing the treeview, I'm not sure this is the best > approach, but to make it expand downwards we need to fix the size of the > dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be > desiderable. Maybe there is a way to keep the dialog resizable and expand the > combobox downwards, but I couldn't find it. > - I've increased the vertical size of the dialog (which I think it was too > small) also to accomodate the upward expansion which otherwise would make the > app tree almost disappear > > > Relevant discussions: > https://bugs.kde.org/show_bug.cgi?id=359233 > https://forum.kde.org/viewtopic.php?f=285=131014 > > > Diffs > - > > src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 > > Diff: https://git.reviewboard.kde.org/r/130180/diff/ > > > Testing > --- > > > Thanks, > > Simone Gaiarin > >
Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
> On Lug. 15, 2017, 7:31 a.m., Simone Gaiarin wrote: > > Inviala! > > Luigi Toscano wrote: > You can't "ship it" you own review. "Ship it" is marked by other > reviewers who thinks that this should go in. > Do you mean to say that you don't have a developer account and someone > else should push this? I'm not very familiar with review board. Indeed I don't have a developer account. I thought that once approved by clicking ship it it would go through, but then I realized what "ship it" is for, so I was just waiting for someone to push it. So yes, I need someone with a developer account to push it please. - Simone --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/#review103453 --- On Lug. 12, 2017, 6:54 p.m., Simone Gaiarin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130180/ > --- > > (Updated Lug. 12, 2017, 6:54 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 359233 > https://bugs.kde.org/show_bug.cgi?id=359233 > > > Repository: kio > > > Description > --- > > The current "open with" dialog implementation does not follow the KDE > principle "Simple by default, powerful when needed" for the following reasons: > - The "run in terminal" and "keep terminal open" options are advanced options > and should not be exposed by default > - The primary goal of the dialog should be to select an application from the > app tree, running command is an advanced feature > > My patch changes the behavior as follow: > > - Put the two options in a KCollapsibleComboBox collapsed by default > - The user can expand it only if he needs to use a command line command > > > Implementation details: > - When the KCollapsibleComboBox is clicked it is expanded upward keeping the > dialog size fixed and compressing the treeview, I'm not sure this is the best > approach, but to make it expand downwards we need to fix the size of the > dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be > desiderable. Maybe there is a way to keep the dialog resizable and expand the > combobox downwards, but I couldn't find it. > - I've increased the vertical size of the dialog (which I think it was too > small) also to accomodate the upward expansion which otherwise would make the > app tree almost disappear > > > Relevant discussions: > https://bugs.kde.org/show_bug.cgi?id=359233 > https://forum.kde.org/viewtopic.php?f=285=131014 > > > Diffs > - > > src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 > > Diff: https://git.reviewboard.kde.org/r/130180/diff/ > > > Testing > --- > > > Thanks, > > Simone Gaiarin > >
Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
> On Lug. 15, 2017, 9:31 a.m., Simone Gaiarin wrote: > > Inviala! You can't "ship it" you own review. "Ship it" is marked by other reviewers who thinks that this should go in. Do you mean to say that you don't have a developer account and someone else should push this? - Luigi --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/#review103453 --- On Lug. 12, 2017, 8:54 p.m., Simone Gaiarin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130180/ > --- > > (Updated Lug. 12, 2017, 8:54 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 359233 > https://bugs.kde.org/show_bug.cgi?id=359233 > > > Repository: kio > > > Description > --- > > The current "open with" dialog implementation does not follow the KDE > principle "Simple by default, powerful when needed" for the following reasons: > - The "run in terminal" and "keep terminal open" options are advanced options > and should not be exposed by default > - The primary goal of the dialog should be to select an application from the > app tree, running command is an advanced feature > > My patch changes the behavior as follow: > > - Put the two options in a KCollapsibleComboBox collapsed by default > - The user can expand it only if he needs to use a command line command > > > Implementation details: > - When the KCollapsibleComboBox is clicked it is expanded upward keeping the > dialog size fixed and compressing the treeview, I'm not sure this is the best > approach, but to make it expand downwards we need to fix the size of the > dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be > desiderable. Maybe there is a way to keep the dialog resizable and expand the > combobox downwards, but I couldn't find it. > - I've increased the vertical size of the dialog (which I think it was too > small) also to accomodate the upward expansion which otherwise would make the > app tree almost disappear > > > Relevant discussions: > https://bugs.kde.org/show_bug.cgi?id=359233 > https://forum.kde.org/viewtopic.php?f=285=131014 > > > Diffs > - > > src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 > > Diff: https://git.reviewboard.kde.org/r/130180/diff/ > > > Testing > --- > > > Thanks, > > Simone Gaiarin > >
Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/#review103453 --- Ship it! Inviala! - Simone Gaiarin On Lug. 12, 2017, 6:54 p.m., Simone Gaiarin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130180/ > --- > > (Updated Lug. 12, 2017, 6:54 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 359233 > https://bugs.kde.org/show_bug.cgi?id=359233 > > > Repository: kio > > > Description > --- > > The current "open with" dialog implementation does not follow the KDE > principle "Simple by default, powerful when needed" for the following reasons: > - The "run in terminal" and "keep terminal open" options are advanced options > and should not be exposed by default > - The primary goal of the dialog should be to select an application from the > app tree, running command is an advanced feature > > My patch changes the behavior as follow: > > - Put the two options in a KCollapsibleComboBox collapsed by default > - The user can expand it only if he needs to use a command line command > > > Implementation details: > - When the KCollapsibleComboBox is clicked it is expanded upward keeping the > dialog size fixed and compressing the treeview, I'm not sure this is the best > approach, but to make it expand downwards we need to fix the size of the > dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be > desiderable. Maybe there is a way to keep the dialog resizable and expand the > combobox downwards, but I couldn't find it. > - I've increased the vertical size of the dialog (which I think it was too > small) also to accomodate the upward expansion which otherwise would make the > app tree almost disappear > > > Relevant discussions: > https://bugs.kde.org/show_bug.cgi?id=359233 > https://forum.kde.org/viewtopic.php?f=285=131014 > > > Diffs > - > > src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 > > Diff: https://git.reviewboard.kde.org/r/130180/diff/ > > > Testing > --- > > > Thanks, > > Simone Gaiarin > >
Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
> On Lug. 14, 2017, 8:24 p.m., David Faure wrote: > > Enlarging the dialog when showing a child widget can sometimes be done by > > activating the toplevel layout, but with a listview above I'm not sure it > > would work. > > Anyway, I like how it behaves with your patch, I find it quite nice > > actually, to make things fit within the window rather than forcefully > > resizing the window. Ok. It seems fine to me as well. - Simone --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/#review103451 --- On Lug. 12, 2017, 6:54 p.m., Simone Gaiarin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130180/ > --- > > (Updated Lug. 12, 2017, 6:54 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 359233 > https://bugs.kde.org/show_bug.cgi?id=359233 > > > Repository: kio > > > Description > --- > > The current "open with" dialog implementation does not follow the KDE > principle "Simple by default, powerful when needed" for the following reasons: > - The "run in terminal" and "keep terminal open" options are advanced options > and should not be exposed by default > - The primary goal of the dialog should be to select an application from the > app tree, running command is an advanced feature > > My patch changes the behavior as follow: > > - Put the two options in a KCollapsibleComboBox collapsed by default > - The user can expand it only if he needs to use a command line command > > > Implementation details: > - When the KCollapsibleComboBox is clicked it is expanded upward keeping the > dialog size fixed and compressing the treeview, I'm not sure this is the best > approach, but to make it expand downwards we need to fix the size of the > dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be > desiderable. Maybe there is a way to keep the dialog resizable and expand the > combobox downwards, but I couldn't find it. > - I've increased the vertical size of the dialog (which I think it was too > small) also to accomodate the upward expansion which otherwise would make the > app tree almost disappear > > > Relevant discussions: > https://bugs.kde.org/show_bug.cgi?id=359233 > https://forum.kde.org/viewtopic.php?f=285=131014 > > > Diffs > - > > src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 > > Diff: https://git.reviewboard.kde.org/r/130180/diff/ > > > Testing > --- > > > Thanks, > > Simone Gaiarin > >
Re: Review Request 130180: Make advanced options of "open with" dialog collabsible and hidden by default
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130180/#review103451 --- Ship it! Enlarging the dialog when showing a child widget can sometimes be done by activating the toplevel layout, but with a listview above I'm not sure it would work. Anyway, I like how it behaves with your patch, I find it quite nice actually, to make things fit within the window rather than forcefully resizing the window. - David Faure On July 12, 2017, 6:54 p.m., Simone Gaiarin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130180/ > --- > > (Updated July 12, 2017, 6:54 p.m.) > > > Review request for KDE Frameworks. > > > Bugs: 359233 > https://bugs.kde.org/show_bug.cgi?id=359233 > > > Repository: kio > > > Description > --- > > The current "open with" dialog implementation does not follow the KDE > principle "Simple by default, powerful when needed" for the following reasons: > - The "run in terminal" and "keep terminal open" options are advanced options > and should not be exposed by default > - The primary goal of the dialog should be to select an application from the > app tree, running command is an advanced feature > > My patch changes the behavior as follow: > > - Put the two options in a KCollapsibleComboBox collapsed by default > - The user can expand it only if he needs to use a command line command > > > Implementation details: > - When the KCollapsibleComboBox is clicked it is expanded upward keeping the > dialog size fixed and compressing the treeview, I'm not sure this is the best > approach, but to make it expand downwards we need to fix the size of the > dialog with setSizeConstraint(QLayout::SetFixedSize); which may not be > desiderable. Maybe there is a way to keep the dialog resizable and expand the > combobox downwards, but I couldn't find it. > - I've increased the vertical size of the dialog (which I think it was too > small) also to accomodate the upward expansion which otherwise would make the > app tree almost disappear > > > Relevant discussions: > https://bugs.kde.org/show_bug.cgi?id=359233 > https://forum.kde.org/viewtopic.php?f=285=131014 > > > Diffs > - > > src/widgets/kopenwithdialog.cpp b28c5b05186bc77413524c99ef61b7967b0ec8b9 > > Diff: https://git.reviewboard.kde.org/r/130180/diff/ > > > Testing > --- > > > Thanks, > > Simone Gaiarin > >
