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.

Sign up for GitHub

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

Jump to bottom

Show quick permission subline on newline #43333

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Show quick permission subline on newline #43333

merged 2 commits into from
Feb 5, 2024

Conversation

Fenn-CS
Copy link
Contributor

@Fenn-CS Fenn-CS commented Feb 4, 2024

Before After
Screenshot from 2024-01-31 00-17-25 Screenshot from 2024-02-04 17-39-31
  • Show quick permission details/explanations on subline
  • Fix icon colors, should be white (or component set color) when highlighted.

@Fenn-CS
Copy link
Contributor Author

Fenn-CS commented Feb 4, 2024

/compile

@Fenn-CS Fenn-CS force-pushed the 43225-fix-share-quick-perms-subline branch 2 times, most recently from 501b3fd to 62d08cd Compare February 4, 2024 21:57
@Fenn-CS
Show quick permission subline on newline
60959db 
- Show quick permission details/explanations on subline
- Fix icon colors, should be white (or component set color) when highlighted.

Signed-off-by: fenn-cs <[email protected]>
@Fenn-CS Fenn-CS force-pushed the 43225-fix-share-quick-perms-subline branch from 62d08cd to 60959db Compare February 4, 2024 22:02
@Fenn-CS
Copy link
Contributor Author

Fenn-CS commented Feb 4, 2024

/compile

@nextcloud-command
chore(assets): Recompile assets
344ca68 
Signed-off-by: nextcloud-command <[email protected]>
@emoral435 emoral435 requested a review from susnux February 5, 2024 13:30
emoral435
emoral435 approved these changes Feb 5, 2024
View reviewed changes
Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

IMO, to me, this change looks way cleaner than having the smaller text right next to the big text. I will cc desginers to get them on the same page, though. cc @nextcloud/designers

szaimen
szaimen approved these changes Feb 5, 2024
View reviewed changes
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.

Yes, looks much nicer from design perspective (didnt test and didnt review the code)

@susnux susnux merged commit 3a556ac into master Feb 5, 2024
97 checks passed
@susnux susnux deleted the 43225-fix-share-quick-perms-subline branch February 5, 2024 15:58
@susnux
Copy link
Contributor

susnux commented Feb 5, 2024

backport?

@Fenn-CS
Copy link
Contributor Author

Fenn-CS commented Feb 5, 2024

backport?

think at least for 28! 27 seems fine.

@Fenn-CS
Copy link
Contributor Author

Fenn-CS commented Feb 5, 2024

/backport to stable28

@backportbot backportbot bot mentioned this pull request Feb 5, 2024
Closed
2 tasks
@blizzz blizzz mentioned this pull request Mar 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux approved these changes

@szaimen szaimen szaimen approved these changes

@emoral435 emoral435 emoral435 approved these changes

@artonge artonge Awaiting requested review from artonge

@Pytal Pytal Awaiting requested review from Pytal

Assignees
No one assigned
Labels
3. to review Waiting for reviews feature: sharing Nice to have
Projects
📁 Files team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom permission subline displays as part of headline on master
5 participants
@Fenn-CS @susnux @szaimen @emoral435 @nextcloud-command