Skip to content

feat(files): add sharing icon in header #40192

Merged
skjnldsv merged 2 commits intomasterfrom
feat/sharing-icon-bread
Sep 4, 2023
Merged

feat(files): add sharing icon in header #40192
skjnldsv merged 2 commits intomasterfrom
feat/sharing-icon-bread

Conversation

@skjnldsv
Copy link
Copy Markdown
Member

@skjnldsv skjnldsv commented Sep 1, 2023

Not shared
2023-09-01_18-10
Shared by link
2023-09-01_18-10_2
Shared
2023-09-01_18-10_1

@skjnldsv skjnldsv self-assigned this Sep 1, 2023
@skjnldsv skjnldsv changed the title feat(files): add uploader feat(files): add sharing icon in header Sep 1, 2023
@skjnldsv skjnldsv mentioned this pull request Sep 1, 2023
26 tasks
@skjnldsv skjnldsv force-pushed the feat/f2v/uploader branch 2 times, most recently from 28b3070 to 52590a7 Compare September 1, 2023 12:35
Base automatically changed from feat/f2v/uploader to master September 1, 2023 12:54
@skjnldsv skjnldsv force-pushed the feat/sharing-icon-bread branch 4 times, most recently from 7f04a0a to 6a91fa3 Compare September 1, 2023 14:05
@skjnldsv skjnldsv requested review from a team, Pytal, artonge, provokateurin and sorbaugh and removed request for a team September 1, 2023 14:05
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Sep 1, 2023
@skjnldsv skjnldsv added this to the Nextcloud 28 milestone Sep 1, 2023
@skjnldsv skjnldsv requested a review from szaimen September 1, 2023 14:13
@szaimen

This comment was marked as resolved.

@provokateurin provokateurin removed their request for review September 1, 2023 15:00
@skjnldsv
Copy link
Copy Markdown
Member Author

skjnldsv commented Sep 1, 2023

@szaimen Feel free to open an issue, but for now this is feature parity.
Please approve so we can move forward :)

EDIT: Unless you can get to an agreement with the design team before it's merged.
But please do not hold back reviews 🙇‍♀️

@tcitworld

This comment was marked as resolved.

@nimishavijay

This comment was marked as resolved.

@skjnldsv

This comment was marked as resolved.

@skjnldsv skjnldsv force-pushed the feat/sharing-icon-bread branch from 6a91fa3 to 088a382 Compare September 1, 2023 16:12
@skjnldsv
Copy link
Copy Markdown
Member Author

skjnldsv commented Sep 1, 2023

The indicator when "not shared" had an opacity of 0.2 to indicate that it's not currently shared, it's just a button to reach the sharing sidebar.

Here you go, we now have exactly what we had on the old files app.
For any change, please open a dedicated issue, let's not discuss this here, thank you :)

@szaimen
Copy link
Copy Markdown
Contributor

szaimen commented Sep 1, 2023

I created #40202

@skjnldsv skjnldsv requested a review from lhsazevedo September 1, 2023 18:11
@szaimen
Copy link
Copy Markdown
Contributor

szaimen commented Sep 1, 2023

I tested this and the icon does not seem to directly update after I shared the folder for example. I guess this should be the case though? Apart from that seems to work well so good work!

@skjnldsv
Copy link
Copy Markdown
Member Author

skjnldsv commented Sep 1, 2023

I tested this and the icon does not seem to directly update after I shared the folder for example. I guess this should be the case though? Apart from that seems to work well so good work!

Needs a separate fix. We'll need to do a pass on global events. But right now the Sidebar doesn't update the files store, you are correct :)

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the feat/sharing-icon-bread branch from 088a382 to 2592568 Compare September 1, 2023 23:16
@skjnldsv skjnldsv enabled auto-merge September 2, 2023 09:12
@szaimen
Copy link
Copy Markdown
Contributor

szaimen commented Sep 4, 2023

I tested this and the icon does not seem to directly update after I shared the folder for example. I guess this should be the case though? Apart from that seems to work well so good work!

Needs a separate fix. We'll need to do a pass on global events. But right now the Sidebar doesn't update the files store, you are correct :)

Shall I create a follow-up issue then?

Copy link
Copy Markdown
Contributor

@lhsazevedo lhsazevedo left a comment

Choose a reason for hiding this comment

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

Working well for me as well

@szaimen
Copy link
Copy Markdown
Contributor

szaimen commented Sep 4, 2023

Shall I create a follow-up issue then?

I've created #40263

Copy link
Copy Markdown
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.

Okay for me then for now

@skjnldsv skjnldsv merged commit fe692f2 into master Sep 4, 2023
@skjnldsv skjnldsv deleted the feat/sharing-icon-bread branch September 4, 2023 22:28
@marcoambrosini marcoambrosini mentioned this pull request Jan 10, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants