Bug 441672 - image forces Kirigami.Card to expand
Summary: image forces Kirigami.Card to expand
Status: REPORTED
Alias: None
Product: frameworks-kirigami
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: Not decided
Assignee: Marco Martin
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-28 19:46 UTC by Matej Starc
Modified: 2023-01-09 14:08 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Files for the result in the image above. (1.35 KB, text/plain)
2021-08-28 19:46 UTC, Matej Starc
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matej Starc 2021-08-28 19:46:25 UTC
Created attachment 141107 [details]
Files for the result in the image above.

SUMMARY
For some reason images that have bigger height that width glitch `Kirigami.Card` so it expands in `Kirigami.CardGridView` which results in columns overlapping.


STEPS TO REPRODUCE
Make a `Kirigami.CardGridView` with more than 4 `Kirigami.Card`'s in it that have image with bigger height that width. This is just an example that i made.

OBSERVED RESULT
Cards in same columns overlap eachother.

EXPECTED RESULT
I don't mind the idea of making a Card expand if the image is bigger but that means it has to actually work and not overlap. I think having some (easy) options for setting image size would be cool in `Kirigami.Card` (if it should expand, fill, ...)

SOFTWARE/OS VERSIONS
Linux/KDE Plasma: 
KDE Plasma Version: 5.22.4
KDE Frameworks Version: 5.85.0
Qt Version: 5.15.2

ADDITIONAL INFORMATION
https://i.imgur.com/j445YVF.png
Comment 1 sh_zam 2023-01-06 13:08:42 UTC
This seems to be due to lack of an attached `Layout.minimumHeight` in https://invent.kde.org/frameworks/kirigami/blob/master/src/controls/templates/AbstractCard.qml#L119-128. I can see a merge request started by you to address this, but I wonder why you didn't proceed with it? https://invent.kde.org/frameworks/kirigami/-/merge_requests/712
Comment 2 Matej Starc 2023-01-06 13:17:01 UTC
(In reply to sh_zam from comment #1)
> This seems to be due to lack of an attached `Layout.minimumHeight` in
> https://invent.kde.org/frameworks/kirigami/blob/master/src/controls/
> templates/AbstractCard.qml#L119-128. I can see a merge request started by
> you to address this, but I wonder why you didn't proceed with it?
> https://invent.kde.org/frameworks/kirigami/-/merge_requests/712

I won't get merged until kf6 or ever. Depends on whay the "og" kirigami devs decide (there was talk of removing CardsGridView completely)
Comment 3 sh_zam 2023-01-09 13:31:19 UTC
“Matej Starc” <bugzilla_noreply@kde.org> writes:

> <https://bugs.kde.org/show_bug.cgi?id=441672>
>
> — Comment #2 from Matej Starc <matej.starc@protonmail.com> —
> (In reply to sh_zam from comment #1)
>> This seems to be due to lack of an attached `Layout.minimumHeight` in
>> <https://invent.kde.org/frameworks/kirigami/blob/master/src/controls/>
>> templates/AbstractCard.qml#L119-128. I can see a merge request started by
>> you to address this, but I wonder why you didn’t proceed with it?
>> <https://invent.kde.org/frameworks/kirigami/-/merge_requests/712>
>
> I won’t get merged until kf6 or ever. Depends on whay the “og” kirigami devs
> decide (there was talk of removing CardsGridView completely)

I think this is more of a bug with Kirigami.Cards than CardsGridView. This bug
is reproducible even if you use the vanilla GridsView.
Comment 4 Matej Starc 2023-01-09 13:46:28 UTC
(In reply to sh_zam from comment #3)
> “Matej Starc” <bugzilla_noreply@kde.org> writes:
> 
> > <https://bugs.kde.org/show_bug.cgi?id=441672>
> >
> > — Comment #2 from Matej Starc <matej.starc@protonmail.com> —
> > (In reply to sh_zam from comment #1)
> >> This seems to be due to lack of an attached `Layout.minimumHeight` in
> >> <https://invent.kde.org/frameworks/kirigami/blob/master/src/controls/>
> >> templates/AbstractCard.qml#L119-128. I can see a merge request started by
> >> you to address this, but I wonder why you didn’t proceed with it?
> >> <https://invent.kde.org/frameworks/kirigami/-/merge_requests/712>
> >
> > I won’t get merged until kf6 or ever. Depends on whay the “og” kirigami devs
> > decide (there was talk of removing CardsGridView completely)
> 
> I think this is more of a bug with Kirigami.Cards than CardsGridView. This
> bug
> is reproducible even if you use the vanilla GridsView.

Its because cards expand out of their cell. This can be either be fixed by limiting the height of the card to the cell height, or allowing the user to customize the cell size (as I've done in my MR).
Comment 5 sh_zam 2023-01-09 13:50:08 UTC
“Matej Starc” <bugzilla_noreply@kde.org> writes:

> <https://bugs.kde.org/show_bug.cgi?id=441672>
>
> — Comment #4 from Matej Starc <matej.starc@protonmail.com> —
> (In reply to sh_zam from comment #3)
>> “Matej Starc” <bugzilla_noreply@kde.org> writes:
>>
>> > <https://bugs.kde.org/show_bug.cgi?id=441672>
>> >
>> > — Comment #2 from Matej Starc <matej.starc@protonmail.com> —
>> > (In reply to sh_zam from comment #1)
>> >> This seems to be due to lack of an attached `Layout.minimumHeight` in
>> >> <https://invent.kde.org/frameworks/kirigami/blob/master/src/controls/>
>> >> templates/AbstractCard.qml#L119-128. I can see a merge request started by
>> >> you to address this, but I wonder why you didn’t proceed with it?
>> >> <https://invent.kde.org/frameworks/kirigami/-/merge_requests/712>
>> >
>> > I won’t get merged until kf6 or ever. Depends on whay the “og” kirigami devs
>> > decide (there was talk of removing CardsGridView completely)
>>
>> I think this is more of a bug with Kirigami.Cards than CardsGridView. This
>> bug
>> is reproducible even if you use the vanilla GridsView.
>
> Its because cards expand out of their cell. This can be either be fixed by
> limiting the height of the card to the cell height, or allowing the user to
> customize the cell size (as I’ve done in my MR).

Yes, in my case I *had* limited the size of the Card to cellHeight, but then the
image (headerItem) expanded into the contentItem (see AbstractCard.qml in
Kirigami) because there is no Layout.minimumHeight for it or anything else for
that matter :)
Comment 6 Matej Starc 2023-01-09 14:08:29 UTC
(In reply to sh_zam from comment #5)
> “Matej Starc” <bugzilla_noreply@kde.org> writes:
> 
> > <https://bugs.kde.org/show_bug.cgi?id=441672>
> >
> > — Comment #4 from Matej Starc <matej.starc@protonmail.com> —
> > (In reply to sh_zam from comment #3)
> >> “Matej Starc” <bugzilla_noreply@kde.org> writes:
> >>
> >> > <https://bugs.kde.org/show_bug.cgi?id=441672>
> >> >
> >> > — Comment #2 from Matej Starc <matej.starc@protonmail.com> —
> >> > (In reply to sh_zam from comment #1)
> >> >> This seems to be due to lack of an attached `Layout.minimumHeight` in
> >> >> <https://invent.kde.org/frameworks/kirigami/blob/master/src/controls/>
> >> >> templates/AbstractCard.qml#L119-128. I can see a merge request started by
> >> >> you to address this, but I wonder why you didn’t proceed with it?
> >> >> <https://invent.kde.org/frameworks/kirigami/-/merge_requests/712>
> >> >
> >> > I won’t get merged until kf6 or ever. Depends on whay the “og” kirigami devs
> >> > decide (there was talk of removing CardsGridView completely)
> >>
> >> I think this is more of a bug with Kirigami.Cards than CardsGridView. This
> >> bug
> >> is reproducible even if you use the vanilla GridsView.
> >
> > Its because cards expand out of their cell. This can be either be fixed by
> > limiting the height of the card to the cell height, or allowing the user to
> > customize the cell size (as I’ve done in my MR).
> 
> Yes, in my case I *had* limited the size of the Card to cellHeight, but then
> the
> image (headerItem) expanded into the contentItem (see AbstractCard.qml in
> Kirigami) because there is no Layout.minimumHeight for it or anything else
> for
> that matter :)

The reason AbstractCard does not limit the height of the header is because there is no need to do it. If you want to limit the header size, you can just limit the size within your code. AbstractCard only offers the standard card layout (with extra functionality).