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

feat(NcListItem): add title slot #5388

Merged
merged 2 commits into from Mar 12, 2024
Merged

Conversation

emoral435
Copy link
Contributor

@emoral435 emoral435 commented Mar 9, 2024

Summary

To extend component flexibility, allow component to be able to render content to the side of name. For instance, in my case, I need to be able to add content to the side of the name for a customer request feature. However, the current implementation keeps the whole line one bolded, and for design reasons, it would be better to allow users of the library to manually style their side content.

Any input on how to better name the slot is appreciate as well :) I did not have any good naming convention to describe content that aligns right to the name of line one

🖼️ Screenshots

🏡 New extension
Code_Ek3YHm9VcZ

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

@emoral435 emoral435 added enhancement New feature or request 3. to review Waiting for reviews component Component discussion and/or suggestion feature: list-item Related to the list-item component labels Mar 9, 2024
@emoral435 emoral435 added this to the 8.10.0 milestone Mar 9, 2024
@emoral435 emoral435 self-assigned this Mar 9, 2024
@emoral435
Copy link
Contributor Author

/backport to next

Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

If we add a new slot here, we need to also check that it works well with compact mode and the new one-line mode and support it.

If this slot is only needed for this specific place, I'd consider this PR as an alternative #3715

So we can provide custom content to name same as currently similar design is implemented in file_versions via #subname.

cc @nextcloud-libraries/designers for a design view here.

@susnux
Copy link
Contributor

susnux commented Mar 11, 2024

If this slot is only needed for this specific place, I'd consider this PR as an alternative #3715
So we can provide custom content to name same as currently similar design is implemented in file_versions via #subname.

This makes sense to me. Would also favor this solution.

@ShGKme ShGKme modified the milestones: 8.10.0, 8.11.0 Mar 11, 2024
@emoral435
Copy link
Contributor Author

This makes sense to me. Would also favor this solution.

After looking at the PR, I agree. This solution is much cleaner, as well! I will manually import the changes that PR introduced but still tag the original PR author.

@emoral435 emoral435 changed the title feat: add right side-content to name feat: add title slot for NcItemList component Mar 11, 2024
@emoral435
Copy link
Contributor Author

I really like this solution: no breaking changes, and styling is consistent with mobile, and with original components.

Mobile with a really long admin name to overflow:
image
Regular:
firefox_8NGoqypHNJ

@emoral435 emoral435 force-pushed the feat/add-listitem-side-content branch from 937ddde to 60226db Compare March 11, 2024 22:10
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

And here we are again 😁

Copy link

@nimishavijay nimishavijay left a comment

Choose a reason for hiding this comment

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

From a design perspective this is a great idea :) there have been multiple times where we've tried to have different styling for the title but it wasn't possible due to the limitations of the component.

I would suggest that text in all the slots be bold by default to maintain consistency.

Signed-off-by: Eduardo Morales <emoral435@gmail.com>
@emoral435 emoral435 force-pushed the feat/add-listitem-side-content branch from 60226db to a6faa9e Compare March 12, 2024 12:21
Signed-off-by: Eduardo Morales <emoral435@gmail.com>
@emoral435 emoral435 force-pushed the feat/add-listitem-side-content branch from a6faa9e to 6287dac Compare March 12, 2024 12:22
@emoral435 emoral435 requested a review from ShGKme March 12, 2024 12:22
@emoral435
Copy link
Contributor Author

I would suggest that text in all the slots be bold by default to maintain consistency.

Fixed with @Antreesy 's PR :D

@emoral435 emoral435 merged commit 01da3ae into master Mar 12, 2024
18 checks passed
@emoral435 emoral435 deleted the feat/add-listitem-side-content branch March 12, 2024 22:32
@emoral435 emoral435 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 13, 2024
@ShGKme ShGKme changed the title feat: add title slot for NcItemList component feat(NcListItem): add title slot Mar 15, 2024
@Antreesy Antreesy mentioned this pull request Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish component Component discussion and/or suggestion enhancement New feature or request feature: list-item Related to the list-item component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants