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

NcListItem: remove max width #5830

Merged
merged 1 commit into from
Jul 23, 2024
Merged

Conversation

GVodyanov
Copy link
Contributor

☑️ Resolves

🖼️ Screenshots

🏚️ Before 🏡 After
image image

@miaulalala

@GVodyanov GVodyanov self-assigned this Jul 18, 2024
@GVodyanov GVodyanov added the 3. to review Waiting for reviews label Jul 18, 2024
@GretaD
Copy link
Contributor

GretaD commented Jul 18, 2024

this will break mail on list layout, most probably.

Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

While the usage in the other PR does not make sense (there is no list so no list item shall be used), I think this makes sense.

I am not 100% sure about the 1-line design as now the name will be much wider but I think its ok(?)

@susnux
Copy link
Contributor

susnux commented Jul 18, 2024

this will break mail on list layout, most probably.

If the name width is important than we should adjust it here by adding the max-width: 300px only for the 1-line design :)

@GVodyanov
Copy link
Contributor Author

this will break mail on list layout, most probably.

@GretaD I did some testing and to me it looks fine actually, worst case scenario if you have a really long title this happens:
image

But as @susnux said we can add the max-width there if needed, makes more sense to have it only there than everywhere I think.

@GretaD
Copy link
Contributor

GretaD commented Jul 19, 2024

But as @susnux said we can add the max-width there if needed, makes more sense to have it only there than everywhere I think.

then lets do that please, add the max-width: 300 for the one line, and remove it for the rest.

@GVodyanov
Copy link
Contributor Author

But as @susnux said we can add the max-width there if needed, makes more sense to have it only there than everywhere I think.

then lets do that please, add the max-width: 300 for the one line, and remove it for the rest.

Done @GretaD :)

src/components/NcListItem/NcListItem.vue Outdated Show resolved Hide resolved
src/components/NcListItem/NcListItem.vue Outdated Show resolved Hide resolved
@GVodyanov GVodyanov force-pushed the style/remove-max-width-list-item branch from 708c228 to 5f39bfd Compare July 22, 2024 13:00
Signed-off-by: Grigory V <scratchx@gmx.com>
@GVodyanov GVodyanov force-pushed the style/remove-max-width-list-item branch from 5f39bfd to dffe5b4 Compare July 23, 2024 08:00
@GVodyanov GVodyanov merged commit 8129292 into master Jul 23, 2024
19 checks passed
@GVodyanov GVodyanov deleted the style/remove-max-width-list-item branch July 23, 2024 08:03
@Antreesy Antreesy added this to the 8.15.1 milestone Jul 23, 2024
@susnux susnux added the bug Something isn't working label Jul 29, 2024
@susnux
Copy link
Contributor

susnux commented Jul 29, 2024

/backport to next

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants