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

sidebar: allow turning off tags view by default #40939

Merged
merged 1 commit into from
Nov 28, 2023
Merged

Conversation

pulsejet
Copy link
Member

With #37065, there is no way to hide the tags from the sidebar by default when they are not relevant or redundant (e.g. the tab may already show the file's tags). This can be annyoing especially when the file has many tags. This patch adds an option to hide the tags from the sidebar by default (the user can still open the tags tab manually).

This also reduces one request when opening the sidebar when the tags are turned off, since all tags don't need to be fetched anymore.

Screenshot: tags are redundant and take up a lot of space.

image

@szaimen szaimen added this to the Nextcloud 28 milestone Oct 17, 2023
@szaimen szaimen added the 3. to review Waiting for reviews label Oct 17, 2023
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

This won't be persisted, right ?

@pulsejet
Copy link
Member Author

This won't be persisted, right ?

Yeah, a reload will switch it back to showing the tags again (current behavior). But I expect the app to call this method to not show the tags on load.

@Pytal
Copy link
Member

Pytal commented Oct 17, 2023

With #37065, there is no way to hide the tags from the sidebar by default when they are not relevant or redundant (e.g. the tab may already show the file's tags).

Worked this way actually since legacy times :)

@pulsejet
Copy link
Member Author

Worked this way actually since legacy times :)

Well there was OCA.SystemTags.View.toggle(). Technically you could combine that with a MutationObserver ...

@pulsejet
Copy link
Member Author

/compile amend /

@Pytal
Copy link
Member

Pytal commented Oct 17, 2023

Well there was OCA.SystemTags.View.toggle(). Technically you could combine that with a MutationObserver ...

🤯, I suppose default was the same then

@pulsejet
Copy link
Member Author

🤯, I suppose default was the same then

Yeah you're right. I think I never noticed it before cause the tags component was rather small 🤔

@pulsejet
Copy link
Member Author

/compile amend /

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@pulsejet
Copy link
Member Author

Any reason this was ignored for 28?

@artonge
Copy link
Contributor

artonge commented Nov 27, 2023

Beside red CI, I see no reason :)

@pulsejet
Copy link
Member Author

/compile amend /

With #37065, there is no way to hide the tags from the sidebar
by default when they are not relevant or redundant (e.g. the tab
may already show the file's tags). This can be annyoing especially
when the file has many tags. This patch adds an option to hide
the tags from the sidebar by default (the user can still open
the tags tab manually).

This also reduces one request when opening the sidebar when the
tags are turned off, since all tags don't need to be fetched
anymore.

Signed-off-by: Varun Patil <varunpatil@ucla.edu>
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@pulsejet
Copy link
Member Author

pulsejet commented Nov 27, 2023

@artonge I've a general question: is it okay if I enable auto-merge on my PRs? I've been not doing the merges myself because I don't work for Nextcloud; not sure how I'm supposed to go about this.

EDIT: Well, this one failed on an unrelated CI error, so I can't merge anyway 🤷🏻(it's not required)

EDIT2: another issue is the quick merge conflict that arises

@artonge
Copy link
Contributor

artonge commented Nov 28, 2023

is it okay if I enable auto-merge on my PRs?

Sure, but some CI steps are not required to auto-merge, so the merge won't get blocked if they don't pass. If you're confident that all tests will pass then go for it :).

EDIT2: another issue is the quick merge conflict that arises

That's hard to do without. But if you have the needed approvals, and if CI failures are not related, ping one of us stating the situation, and we'll force merge for you.

@artonge
Copy link
Contributor

artonge commented Nov 28, 2023

You can also try to restart some failing CI steps. Note that for cypress, you need to restart all the steps and not only the failing ones.

@artonge artonge merged commit a61ec60 into master Nov 28, 2023
42 checks passed
@artonge artonge deleted the pulsejet/sidebar-tags branch November 28, 2023 12:43
@pulsejet
Copy link
Member Author

/backport to stable28

@skjnldsv
Copy link
Member

/skjnldsv-backport to stable28

@solracsf
Copy link
Member

solracsf commented Jul 4, 2024

/backport to stable28

Copy link

backportbot bot commented Jul 4, 2024

The backport to stable28 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable28
git pull origin stable28

# Create the new backport branch
git checkout -b backport/40939/stable28

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick c5810245

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/40939/stable28

Error: Failed to push branch backport/40939/stable28: fatal: could not read Username for 'https://github.com': No such device or address


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@solracsf
Copy link
Member

solracsf commented Jul 5, 2024

/backport 424eb12 to stable28

Copy link

backportbot bot commented Jul 5, 2024

The backport to stable28 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable28
git pull origin stable28

# Create the new backport branch
git checkout -b backport/40939/stable28

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 424eb124

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/40939/stable28

Error: Failed to cherry pick commits: error: no cherry-pick or revert in progress
fatal: cherry-pick failed


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

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.

7 participants