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

NcAppSidebar: fix tabs with css icon #4111

Merged
merged 2 commits into from May 16, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented May 13, 2023

Resolves

This PR resolves two different problems in NcSidebarTab with CSS icon. I combined them together into 1 PR because they both required to be fixed to resolve the problem.

1. Resolves rendering by OCA.Files.Sidebar

Fixes: nextcloud/server#38229
Regression of: #3891

The icon is rendered by NcAppSidebarTab.renderIcon() in NcAppSidebarTabs. It seems to me when it is used with OCA.Files.Sidebar (and maybe LegacyView), Tabs and Tab components are rendered by different Vue copies on the page (on my instance 24 different copies of Vue are bundled).

As a result, when NcAppSidebarTab.renderIcon() is called in NcAppSidebarTabs's rendering, it doesn't know about the rendering. NcAppSidebarTab.renderIcon() creates VNode in one Vue copy, but is used in rendering in another Vue copy. It fails, because for NcAppSidebarTab.renderIcon() there is no rendering in the first Vue copy.

In this PR rendering of the icon is simplified. NcAppSIdebarTab.renderIcon() is used ONLY to render the slot's icon. CSS icon is rendered by NcSidebarTabs directly as a fallback. Then it works fine in different contexts.

P.S. To fix the mentioned issue this PR should be manually backported: #4112

2. Resolves losing CSS icon

Fixes the problem that causes the problem fixed in: nextcloud/activity#1154
Also fix: nextcloud/activity#1167
Regression of: #3945

During the update to the new design with NcCheckboxRadioSwitch, styles for CSS icons were lost. Span with CSS icon had width 0.

This PR returns the styles with the default size 20px as used in NcCheckboxRadioSwitch.

@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews feature: app-sidebar Related to the app-sidebar component labels May 13, 2023
@ShGKme ShGKme self-assigned this May 13, 2023
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>

wip

Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/render-tab-in-tabs-without-context branch from ac495b7 to b6c8df4 Compare May 13, 2023 22:34
@ShGKme ShGKme added this to the 8.0.0 milestone May 13, 2023
@ShGKme
Copy link
Contributor Author

ShGKme commented May 13, 2023

Tested on server/stable26 with activity/stable26 on the Files sidebar.

image

Copy link
Contributor

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

Also fixes the activity app

@nickvergessen nickvergessen merged commit 411d059 into master May 16, 2023
16 checks passed
@nickvergessen nickvergessen deleted the fix/render-tab-in-tabs-without-context branch May 16, 2023 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: app-sidebar Related to the app-sidebar component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Right sidebar has no tab headers anymore on the files app when activity app is enabled
3 participants