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

Do not show tab headers marked as hidden #12570

Merged
merged 1 commit into from Nov 21, 2018

Conversation

@danxuliu
Copy link
Member

@danxuliu danxuliu commented Nov 21, 2018

This pull request fixes a regression introduced in #12180

Tab headers that should not be shown are marked with the hidden CSS class. The CSS rules set display: none for .hidden elements, but as the rules for .tabHeaders .tabHeader are more specific than rules for .hidden the display property is overriden and ends being flex. Therefore, it is necessary to explicitly set a rule for .tabHeaders .tabHeader.hidden elements.

Before:
sidebar-hidden-tab-before

After:
sidebar-hidden-tab-after

@nextcloud/designers

Tab headers that should not be shown are marked with the "hidden" CSS
class. The CSS rules set "display: none" for ".hidden" elements, but as
the rules for ".tabHeaders .tabHeader" are more specific than rules for
".hidden" the "display" property is overriden and ends being "flex".
Therefore, it is necessary to explicitly set a rule for ".tabHeaders
.tabHeader.hidden" elements.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 21, 2018

Seems fair to me!
Question: how do you end up with a hidden tab? Why would the tab be disabled? like version for a folder is not possible for example? (simple example, I have actually no idea 🤔 ) 😄

@danxuliu
Copy link
Member Author

@danxuliu danxuliu commented Nov 21, 2018

Question: how do you end up with a hidden tab? Why would the tab be disabled?

When the Files app is initialized all the sidebar plugins are registered and then, every time a file is selected, the plugins are shown or hidden depending on the current file.

like version for a folder is not possible for example? (simple example, I have actually no idea thinking )

No, versions are not supported in folders (or so it seems :-P ).

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 21, 2018

That's what I thought.
What a terrible behavior :(

@danxuliu
Copy link
Member Author

@danxuliu danxuliu commented Nov 21, 2018

What a terrible behavior :(

I am curious :-) What would you suggest instead?

@jancborchardt
Copy link
Member

@jancborchardt jancborchardt commented Nov 21, 2018

Let’s open a separate issue about this discussion? :) This fixes a regression bug, and the discussion is opening a whole new topic of implementation.

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 21, 2018

What a terrible behavior :(

I am curious :-) What would you suggest instead?

Let's go over #10289

@danxuliu
Copy link
Member Author

@danxuliu danxuliu commented Nov 21, 2018

Let’s open a separate issue about this discussion? :) This fixes a bug, and the discussion is opening a whole new topic of implementation.

Oh, it was just simple curiosity, I did not intend to discuss anything :-) But yes, I should have asked @skjnldsv by other channels, sorry :-)

@rullzer rullzer merged commit 8c9e78d into master Nov 21, 2018
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/drone/pr the build failed
Details
@dco
DCO DCO
Details
@fixupbot
fixupbot No fixup commits found. The commit history is clean
Details
@rullzer rullzer deleted the do-not-show-tab-headers-marked-as-hidden branch Nov 21, 2018
@MorrisJobke MorrisJobke mentioned this pull request Nov 22, 2018
7 of 7 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants