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

Hidden Tab in Tabsheet lets surrounding tabs stick to each other #10437

Closed
focbenz opened this issue Dec 13, 2017 · 14 comments · Fixed by #12078
Closed

Hidden Tab in Tabsheet lets surrounding tabs stick to each other #10437

focbenz opened this issue Dec 13, 2017 · 14 comments · Fixed by #12078
Labels
bug workaround Workaround provided in ticket comments

Comments

@focbenz
Copy link
Contributor

focbenz commented Dec 13, 2017

If one tab is hidden inside a TabSheet the space between the surrounding tabs is less than between two normal tabs.
We would consider this a cosmetic issue / bug - or is there a reasoning. e.g. should it be visible that a tab was hidden ? (old thread on the Vaadin forum: https://vaadin.com/forum/#!/thread/15553609)

Created with the code from the TabSheet sampler.
https://vaadin.com/docs/-/part/framework/layout/layout-tabsheet.html

Sample with Vaadin 8.1.7:

vaadin8hiddentabspacev817

Sample project with Vaadin 8.8.5: https://github.com/FOCONIS/tabsheetdemov8hidden

Sample code:

final VerticalLayout outerlayout = new VerticalLayout();
outerlayout.addComponent(new Label("Tabsheet Hidden Tab Test with Vaadin Version: " + 
		com.vaadin.shared.Version.getFullVersion()));

TabSheet sample = new TabSheet();
sample.setHeight(100.0f, Unit.PERCENTAGE);
sample.addStyleName(ValoTheme.TABSHEET_FRAMED);
sample.addStyleName(ValoTheme.TABSHEET_PADDED_TABBAR);

for (int i = 1; i < 8; i++) {
	final Label label = new Label("The 5th tab is hidden and both side tabs 'stick' together."
			+ "Demonstrating the issue on <a href='https://vaadin.com/forum/#!/thread/15553609'>Vaadin Forum</a>", ContentMode.HTML);
	label.setWidth(100.0f, Unit.PERCENTAGE);

	final VerticalLayout layout = new VerticalLayout(label);
	layout.setMargin(true);
	
	Tab currTab = sample.addTab(layout, "Tab " + i);
	
	if (i == 5) { // hide the 5th tab
	  currTab.setVisible(false);
	}
}

outerlayout.addComponent(sample);
outerlayout.setMargin(true);
outerlayout.setSpacing(true);

setContent(outerlayout);
@anasmi
Copy link
Contributor

anasmi commented Mar 19, 2018

Hi,
It looks like for some reason the style of the first tab is passed to the one after the hidden one. By default margin-left is 4px, for every tabs, but not the first one. After hiding one of the tabs, this style is wrongly applied to another tab. I don't know the root case and don't have time to verify further, but I don't believe it's intended.
I hope the picture below explains better, what I mean:
tabsheetstyleissue

@sebdavid
Copy link

sebdavid commented Jun 26, 2018

Hi,
I have the same issue with Vaadin 7.7.13 with the following case (tested on Firefox 60 and Chrome 67).
When tab B is hidden, there is no spacing between tabs A and C.

TabSheet tabs = new TabSheet();
tabs.addTab(new Label("page A"), "Tab A");
tabs.addTab(new Label("page B"), "Tab B").setVisible(false);
tabs.addTab(new Label("page C"), "Tab C");

TabSheet tabs2 = new TabSheet();
tabs2.addTab(new Label("page A"), "Tab A");
tabs2.addTab(new Label("page B"), "Tab B");
tabs2.addTab(new Label("page C"), "Tab C");

The result is :
image

Regarding the applied CSS style, it seems that the following rule is in cause :

.v-tabsheet-tabitemcell:first-child .v-caption, 
.v-tabsheet-tabitemcell[aria-hidden="true"] + td .v-caption {
    margin-left: 0;
}

.v-tabsheet-tabitemcell[aria-hidden="true"] is matching the disabled tab, and + td .v-caption matches the caption of the following element.

@sebdavid
Copy link

As a workaround, I am overriding the rules with the following in my .scss file :

	.v-tabsheet-tabitemcell{
		&[aria-hidden="true"] + td .v-caption{
			margin-left: round($v-unit-size/2);
		}
	}
	.v-tabsheet-framed > .v-tabsheet-tabcontainer{
		[aria-hidden="true"] + td .v-caption{
			margin-left: round($v-unit-size/10);
		}
	}

But I don't know if there will be a side effect. Actually, I am wondering what was the original purpose of the rule .v-tabsheet-tabitemcell[aria-hidden="true"] + td .v-caption{ margin-left: 0 }. Maybe to fix something on a particular browser?

@stale
Copy link

stale bot commented Nov 23, 2018

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

@stale stale bot added the Stale Stale bot label label Nov 23, 2018
@focbenz
Copy link
Contributor Author

focbenz commented Nov 26, 2018

Oh, no ... there seems not to be too much interest in fixing this.

@stale stale bot removed the Stale Stale bot label label Nov 26, 2018
@stale
Copy link

stale bot commented Apr 25, 2019

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

@stale stale bot added the Stale Stale bot label label Apr 25, 2019
@focbenz
Copy link
Contributor Author

focbenz commented Apr 25, 2019

Still an issue ... might still be a fix worth including in framework 8.

@stale stale bot removed the Stale Stale bot label label Apr 25, 2019
@stale
Copy link

stale bot commented Sep 22, 2019

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

@stale stale bot added the Stale Stale bot label label Sep 22, 2019
@focbenz
Copy link
Contributor Author

focbenz commented Sep 24, 2019

Issue still present in Vaadin 8.8.5

Sample project: https://github.com/FOCONIS/tabsheetdemov8hidden

@stale stale bot removed the Stale Stale bot label label Sep 24, 2019
@stale
Copy link

stale bot commented Feb 21, 2020

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

@stale stale bot added the Stale Stale bot label label Feb 21, 2020
@focbenz
Copy link
Contributor Author

focbenz commented Feb 21, 2020

Should be a simple CSS fix as provided by sebdavid

@stale stale bot removed the Stale Stale bot label label Feb 21, 2020
@TatuLund TatuLund added workaround Workaround provided in ticket comments bug labels Feb 22, 2020
@stale
Copy link

stale bot commented Jul 25, 2020

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

@stale stale bot added the Stale Stale bot label label Jul 25, 2020
@focbenz
Copy link
Contributor Author

focbenz commented Aug 14, 2020

Stale but still good to be fixed.

@stale stale bot removed the Stale Stale bot label label Aug 14, 2020
Ansku added a commit to Ansku/framework that referenced this issue Aug 17, 2020
@Ansku
Copy link
Member

Ansku commented Aug 17, 2020

Thanks for being persistent, the previous reminder came just after I left for vacation and I completely missed it in my backlog 🙈 Unfortunately the issue doesn't seem to be quite as trivial as that. I made an experimental pull request to see what our validation builds think of the matter, and it turns out that the lack of margin is very much required when the TabSheet has too many tabs to fit the view and at least one of them is hidden because of scrolling. Haven't quite figured out yet how to get the margin there only for the cases when there are also visible tabs to the left of the hidden tabs.

Ansku added a commit to Ansku/framework that referenced this issue Aug 18, 2020
Ansku added a commit to Ansku/framework that referenced this issue Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug workaround Workaround provided in ticket comments
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants