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

chore(NcNoteCard): use custom icon slot #11545

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Feb 7, 2024

☑️ Resolves

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

I just reversed the Chevron because the div gets up and space was reduced.

🏚️ Before 🏡 After
image image
|

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences

@DorraJaouad DorraJaouad added the feature: frontend 🖌️ "Web UI" client label Feb 7, 2024
@DorraJaouad DorraJaouad added this to the 💞 Next Major (29) milestone Feb 7, 2024
@DorraJaouad DorraJaouad requested a review from Antreesy February 7, 2024 10:28
@DorraJaouad DorraJaouad self-assigned this Feb 7, 2024
@Antreesy
Copy link
Contributor

Antreesy commented Feb 7, 2024

if I remember, @szaimen has some thoughts on this component design

@szaimen
Copy link
Contributor

szaimen commented Feb 7, 2024

if I remember, @szaimen has some thoughts on this component design

Yes, please make it look like this: nextcloud-libraries/nextcloud-vue#4894 (comment)

@DorraJaouad
Copy link
Contributor Author

DorraJaouad commented Feb 7, 2024

Yes, please make it look like this: nextcloud-libraries/nextcloud-vue#4894 (comment)

So to indicate the requested changes (cc @szaimen) :

  • Remove Chevron icon
  • Use short message of the absence data
  • Make the avatar in default size

@szaimen
Copy link
Contributor

szaimen commented Feb 7, 2024

  • Remove Chevron icon

The chevron should only be shown if there is a longer message available so that you can hide it. The short message should always be shown.

@DorraJaouad DorraJaouad force-pushed the chore/noid/use-custom-icon-slot branch from a284fc3 to a73efd5 Compare February 8, 2024 12:19
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.

All good, let's backport it to 28

@szaimen
Copy link
Contributor

szaimen commented Feb 8, 2024

May I ask how it looks now?

@Antreesy
Copy link
Contributor

Antreesy commented Feb 8, 2024

May I ask how it looks now?

Screenshots seem to be actual, though you had different border-radius on your sketch?

Screencast.from.08.02.2024.23.20.22.webm

@szaimen
Copy link
Contributor

szaimen commented Feb 9, 2024

Thanks @Antreesy! Before merging, please fix the look of the left side. Please compare:

How it currently looks How it should look
image image

Signed-off-by: DorraJaouad <dorra.jaoued7@gmail.com>
@DorraJaouad DorraJaouad force-pushed the chore/noid/use-custom-icon-slot branch from a73efd5 to 5da4fa5 Compare February 9, 2024 12:50
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Screenshot looks good now, thanks :)

@DorraJaouad DorraJaouad merged commit 357aeb0 into main Feb 9, 2024
37 checks passed
@DorraJaouad DorraJaouad deleted the chore/noid/use-custom-icon-slot branch February 9, 2024 13:12
@Antreesy
Copy link
Contributor

/backport to stable28

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants