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

Revert "Improve shared status button" #42041

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Revert "Improve shared status button" #42041

wants to merge 1 commit into from

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Dec 5, 2023

This reverts commit c2c5994.

Sorry @marcoambrosini, I cannot approve the previous PR:

  1. using deep selectors is not acceptable to me
  2. you're changing this for all actions, while only the sharing got request
  3. a comment was written by a lead with another lead reaction and got completely ignored

I'll have a look with the design team to make sure we keep the previous issue fixed too

@skjnldsv skjnldsv added this to the Nextcloud 28 milestone Dec 5, 2023
@skjnldsv skjnldsv requested review from marcoambrosini, a team, Fenn-CS, sorbaugh, emoral435 and susnux and removed request for a team December 5, 2023 16:40
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 5, 2023
@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 5, 2023

I'll have a look with the design team to make sure we keep the previous issue fixed too

@blizzz blizzz mentioned this pull request Dec 5, 2023
5 tasks
@marcoambrosini
Copy link
Member

Aside from the execution, admittedly poor and rushed, I just want to register that I did this reluctantly and didn't agree much with the set of changes proposed for this button. Nor with how fast they were rushed in without exploring other solutions.
So I thought of this as a temporary hack, while waiting to have have shared separate shared and owner columns.

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 6, 2023

All good, Marco :)
if the button style must go, then it must go! But without global styling

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.

I'll have a look with the design team to make sure we keep the previous issue fixed too

Could we make sure that we don’t revert it without addressing that part here already?
The specification / mockup exists at #37208, and as long as design-wise it looks like that, you are expert regarding implementation. :)

@skjnldsv
Copy link
Member Author

skjnldsv commented Dec 6, 2023

Could we make sure that we don’t revert it without addressing that part here already?

image

This was referenced Dec 8, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 28.0.1 Dec 11, 2023
This was referenced Dec 15, 2023
@blizzz blizzz mentioned this pull request Jan 15, 2024
@blizzz
Copy link
Member

blizzz commented Jan 17, 2024

What about this PR, it appears stale?

@skjnldsv skjnldsv marked this pull request as draft January 18, 2024 11:26
@skjnldsv skjnldsv mentioned this pull request Feb 14, 2024
8 tasks
@skjnldsv skjnldsv removed this from the Nextcloud 28.0.3 milestone Feb 14, 2024
@emoral435 emoral435 removed their request for review March 4, 2024 16:11
@susnux susnux added this to the Nextcloud 30 milestone Apr 18, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants