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(files): add folder icon overlay #40209

Merged
merged 1 commit into from
Sep 5, 2023
Merged

feat(files): add folder icon overlay #40209

merged 1 commit into from
Sep 5, 2023

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Sep 1, 2023

Ref #39914

I decided to hardcode it for now as I couldn't find a good API way to tackle that.
We can always talk and improve from there :)

image

@nextcloud/designers if you want to adjust those icons, suggest new ones from the material design set as a review comment. I currently used the ones we already use or some that got asked by Jan.

@skjnldsv skjnldsv added this to the Nextcloud 28 milestone Sep 1, 2023
@skjnldsv skjnldsv requested a review from a team September 1, 2023 23:01
@skjnldsv skjnldsv self-assigned this Sep 1, 2023
@skjnldsv skjnldsv requested review from artonge, szaimen and sorbaugh and removed request for a team September 1, 2023 23:01
@skjnldsv skjnldsv mentioned this pull request Sep 1, 2023
26 tasks
@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 1, 2023

I don't have an e2ee setup available, can you try it yourself @artonge ? 🙏

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.

LGTM but didnt test

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks good design-wise!

It would be nice to switch the share-triangle to this less technical "add person" icon:
https://pictogrammers.com/library/mdi/icon/account-plus/
I think I mentioned to you before @skjnldsv? But probably separate issue as it should be done across the board – your pick.

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 5, 2023

I think I mentioned to you before @skjnldsv?

We did that for the Navigation yes
Do I understand you want me to replace them every time I encounter the old ones?

@skjnldsv

This comment was marked as resolved.

@artonge

This comment was marked as resolved.

@skjnldsv

This comment was marked as resolved.

@artonge

This comment was marked as resolved.

@skjnldsv

This comment was marked as resolved.

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 5, 2023

Before After
image image

@nextcloud/designers, without too much discussions, since we also have the locked state for a file/folder, I figured the key for encrypted was more suitable?
Please advise, else I can quickly revert it to the lock :)

@szaimen
Copy link
Contributor

szaimen commented Sep 5, 2023

Yes the key looks good to me 👍

cc @jancborchardt @nimishavijay

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@jancborchardt
Copy link
Member

Yup key for encrypted folder and lock for locked folder is good! @tobiasKaminsky this probably needs to be reflected on the mobile clients as well?

@skjnldsv skjnldsv removed the 3. to review Waiting for reviews label Sep 5, 2023
@skjnldsv skjnldsv added the 4. to release Ready to be released and/or waiting for tests to finish label Sep 5, 2023
@skjnldsv skjnldsv merged commit f2787c0 into master Sep 5, 2023
35 of 39 checks passed
@skjnldsv skjnldsv deleted the feat/overlayicon branch September 5, 2023 12:43
@tobiasKaminsky
Copy link
Member

Yup key for encrypted folder and lock for locked folder is good! @tobiasKaminsky this probably needs to be reflected on the mobile clients as well?

I created an issue for us, so that we an update all folder icons (e.g. this "unsplash" thing is also new to me).

@skjnldsv
Copy link
Member Author

skjnldsv commented Sep 5, 2023

Yep, icons are changing more and more along the releases.
As we're moving them all to MaterialDesign

@tobiasKaminsky
Copy link
Member

Once we start, we will ask/check for a complete list, so that all clients have same as web.

@allexzander
Copy link

@skjnldsv Where can I get the exact same icons as in your screenshot?

@skjnldsv
Copy link
Member Author

skjnldsv commented Oct 3, 2023

@skjnldsv Where can I get the exact same icons as in your screenshot?

https://materialdesignicons.com/

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 enhancement feature: files
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants