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

style(TopBar): fit two-lines with normal line-height #12851

Merged
merged 1 commit into from
Aug 1, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions src/components/TopBar/TopBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -303,17 +303,21 @@ export default {

<style lang="scss" scoped>
.top-bar {
--border-width: 1px;
// hardcoded 1.5 value of line-height for compatibility with older versions
--text-height: calc(1.5 * (var(--default-font-size) + var(--font-size-small, 15px)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why default small size to 15px? Do we want to backport it to 29?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Font size shouldn't change for Desktop client against 29, am I right?

Copy link
Contributor

Choose a reason for hiding this comment

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

It will use server styles required for the current Talk version.

Otherwise it's more painful and we would need support not only different backend versions but also styles on frontend

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please adjust in your follow up, when possible. I'm not getting it fully still 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Was 15px added here only for the Desktop client?

Copy link
Contributor

Choose a reason for hiding this comment

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

We adjusted in libraries like @nextcloud/vue because it is used on multiple versions.

Did we do it on Talk?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR is not backported and adapts to new styles and variables, which is necessary for Talk web on 30 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant he iconSize, which depends on server styles and not the Talk version

42e9191#diff-841f2c9a1004d8fafe1e1f6fe44412b062860ea277a4ddc071068f7f2a162e7dR156-R157

Copy link
Contributor

@ShGKme ShGKme Aug 4, 2024

Choose a reason for hiding this comment

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

It's better to get it from variable anyway. Same as with other constant values in styles.

display: flex;
flex-wrap: wrap;
z-index: 10;
gap: 3px;
align-items: flex-start;
align-items: center;
justify-content: flex-end;
padding: calc(2 * var(--default-grid-baseline));
min-height: calc(2 * var(--default-grid-baseline) + var(--text-height) + var(--border-width));
padding-block: var(--default-grid-baseline);
// Reserve space for the sidebar toggle button
padding-right: calc(2 * var(--default-grid-baseline) + var(--app-sidebar-offset, 0));
padding-inline: calc(2 * var(--default-grid-baseline)) calc(2 * var(--default-grid-baseline) + var(--app-sidebar-offset, 0));
background-color: var(--color-main-background);
border-bottom: 1px solid var(--color-border);
border-bottom: var(--border-width) solid var(--color-border);

.talk-sidebar-callview & {
margin-right: var(--default-clickable-area);
Expand Down Expand Up @@ -358,8 +362,6 @@ export default {
justify-content: center;
width: 100%;
overflow: hidden;
min-height: var(--default-clickable-area);
line-height: calc(var(--default-clickable-area) / 2);
&--offline {
color: var(--color-text-maxcontrast);
}
Expand All @@ -370,6 +372,7 @@ export default {
text-overflow: ellipsis;
}
.description {
font-size: var(--font-size-small, 15px);
overflow: hidden;
text-overflow: ellipsis;
max-width: fit-content;
Expand Down
Loading