Skip to content

Updated VBtn to KButton for New Collection - #5275

Merged
MisRob merged 13 commits into
learningequality:unstablefrom
yeshwanth235:issue-5243
Aug 25, 2025
Merged

MisRob merged 13 commits into
learningequality:unstablefrom
yeshwanth235:issue-5243

Conversation

@yeshwanth235

Copy link
Copy Markdown
Contributor

Summary

Updated VBtn to KButton for New Collection

image image

References

5243

Reviewer guidance

Test buttons in Channels > Collections > New collection.
Should working as expected.

@MisRob
MisRob self-requested a review August 13, 2025 02:58
@MisRob MisRob self-assigned this Aug 13, 2025
@MisRob

MisRob commented Aug 14, 2025

Copy link
Copy Markdown
Member

Thanks @yeshwanth235, I will review next week.

@MisRob MisRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @yeshwanth235, all looks good. Just small cleanup and we can merge.

Note that we may need to wait few days before merging because of pre-release preparations.

>
{{ $tr('finish') }}
</VBtn>
<div>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It doesn't seem this <div> serves any function, and it wasn't there before either. Let's cleaned it up, and then we can merge. Thanks!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Without div the footer looks like this.

image

With div

image

Hence added the div

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the div and added a class to kButton to adjust the height.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah that's unusual. Do you know why the button was spreading that way?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The CSS for KButton includes a min-height property. Because the footer has a greater height, the button’s height has increased to match it.

The button’s min-height ensures a minimum size, but parent or surrounding elements can cause it to appear taller.

image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah okay, thank you. There's probably some style in the bottom bar, perhaps flex, that affects its height.

In that case setting the height explicitly will be better than applying that div - it makes clearer to others what's going on.

@MisRob MisRob left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Everything as expected and I manually confirmed that buttons work just fine. Thank you @yeshwanth235 :)

@MisRob
MisRob merged commit 6bf2411 into learningequality:unstable Aug 25, 2025
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.

2 participants