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

WEBUI-362: fix tabs height #1260

Merged
merged 1 commit into from Aug 12, 2021
Merged

WEBUI-362: fix tabs height #1260

merged 1 commit into from Aug 12, 2021

Conversation

semisse
Copy link
Collaborator

@semisse semisse commented Jul 20, 2021

No description provided.

@semisse semisse requested a review from a team as a code owner July 20, 2021 13:59
@nuxeojenkins
Copy link
Contributor

View issue in JIRA: WEBUI-362: Fix double scrollbars on listings in Web UI

@semisse semisse requested review from Gabez0r and mnixo July 20, 2021 13:59
Gabez0r
Gabez0r reviewed Jul 20, 2021
View reviewed changes
Copy link
Collaborator

@Gabez0r Gabez0r left a comment

Choose a reason for hiding this comment

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

looking good

elements/nuxeo-browser.html Outdated
@@ -40,12 +40,16 @@

paper-tabs {
--paper-tabs-selection-bar-color: transparent;
height: auto;
max-height: 42px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it need to be a max-height instead of a height?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was trying to avoid having a hardcoded height here, but it won't make a huge difference in this case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm ok with either way, was just trying to understand if there was any reason in particular. For example, if there are no tabs being contributed, what happens with one or the other?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In that edge case, we should go with max-height and make sure height is set to auto so the tab bar won't be visible. Just updated the PR to reflect this.

@semisse semisse force-pushed the fix-WEBUI-362-fixTabsHeight branch from 7c8c487 to 6690922 Compare July 20, 2021 14:48
@semisse semisse requested review from Gabez0r and a team July 20, 2021 14:50
@nuxeo-web-ui-jx-bot
Copy link
Contributor

⭐ PR built and available in a preview environment nuxeo-nuxeo-web-ui-pr-1260 here

1 similar comment
@nuxeo-web-ui-jx-bot
Copy link
Contributor

⭐ PR built and available in a preview environment nuxeo-nuxeo-web-ui-pr-1260 here

mnixo
mnixo previously approved these changes Jul 21, 2021
View reviewed changes
@semisse semisse force-pushed the fix-WEBUI-362-fixTabsHeight branch 2 times, most recently from 6754167 to 33bc4f8 Compare July 27, 2021 11:15
@semisse semisse requested a review from mnixo July 27, 2021 11:16
@semisse semisse force-pushed the fix-WEBUI-362-fixTabsHeight branch 2 times, most recently from 8e073e8 to 424ecd7 Compare July 27, 2021 11:24
@nuxeo-web-ui-jx-bot
Copy link
Contributor

⭐ PR built and available in a preview environment nuxeo-nuxeo-web-ui-pr-1260 here

mnixo
mnixo previously approved these changes Jul 27, 2021
View reviewed changes
richardsd
richardsd previously approved these changes Aug 2, 2021
View reviewed changes
Gabez0r
Gabez0r previously approved these changes Aug 2, 2021
View reviewed changes
@semisse semisse dismissed stale reviews from Gabez0r, richardsd, and mnixo via 3069831 August 12, 2021 15:15
@semisse semisse force-pushed the fix-WEBUI-362-fixTabsHeight branch from 424ecd7 to 3069831 Compare August 12, 2021 15:15
@semisse
Copy link
Collaborator Author

semisse commented Aug 12, 2021

I suggest we go with min-height for tabs to ensure they have space to increase their height if needed. Since having no tabs is probably an edge case, let's keep it as it is. If needed, later we can always check if the paper-tabs have children being contributed and make it display none in the UI.

@semisse semisse requested a review from richardsd August 12, 2021 15:19
@semisse semisse requested review from Gabez0r and mnixo August 12, 2021 15:19
@nuxeo-web-ui-jx-bot
Copy link
Contributor

⭐ PR built and available in a preview environment nuxeo-nuxeo-web-ui-pr-1260 here

@semisse semisse merged commit 7293462 into master Aug 12, 2021
@semisse semisse deleted the fix-WEBUI-362-fixTabsHeight branch August 12, 2021 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardsd richardsd richardsd approved these changes

@mnixo mnixo mnixo approved these changes

@Gabez0r Gabez0r Gabez0r approved these changes

@nelsonsilva nelsonsilva Awaiting requested review from nelsonsilva

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
6 participants
@semisse @nuxeojenkins @nuxeo-web-ui-jx-bot @richardsd @mnixo @Gabez0r