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

Migrate/to nextcloud vue 8 #5648

Merged
merged 1 commit into from
Feb 19, 2024
Merged

Migrate/to nextcloud vue 8 #5648

merged 1 commit into from
Feb 19, 2024

Conversation

GVodyanov
Copy link
Contributor

@GVodyanov GVodyanov commented Dec 28, 2023

Fix #5619

Progress:

  • Change title to name
  • Change MultiSelect to Select props
  • Work out errors with Select when multiple = true

@GVodyanov GVodyanov linked an issue Dec 28, 2023 that may be closed by this pull request
4 tasks
Copy link

codecov bot commented Dec 28, 2023

Codecov Report

Attention: 69 lines in your changes are missing coverage. Please review.

Comparison is base (37b2047) 24.50% compared to head (fd355c9) 24.48%.
Report is 9 commits behind head on main.

Files Patch % Lines
src/components/Shared/CalendarPicker.vue 0.00% 10 Missing ⚠️
...nents/Editor/Properties/PropertySelectMultiple.vue 0.00% 8 Missing ⚠️
...Properties/PropertySelectMultipleColoredOption.vue 0.00% 8 Missing ⚠️
src/components/AppNavigation/Settings.vue 0.00% 5 Missing ⚠️
...c/components/Editor/Resources/ResourceRoomType.vue 0.00% 5 Missing ⚠️
...c/components/Editor/Resources/ResourceListItem.vue 0.00% 4 Missing ⚠️
...components/Editor/Resources/ResourceListSearch.vue 0.00% 4 Missing ⚠️
.../components/Editor/Invitees/InviteesListSearch.vue 0.00% 3 Missing ⚠️
...rc/components/Editor/Properties/PropertySelect.vue 0.00% 3 Missing ⚠️
...NavigationHeader/AppNavigationHeaderDatePicker.vue 0.00% 2 Missing ⚠️
... and 14 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5648      +/-   ##
============================================
- Coverage     24.50%   24.48%   -0.03%     
  Complexity      423      423              
============================================
  Files           243      243              
  Lines         10944    10954      +10     
  Branches       1798     1803       +5     
============================================
  Hits           2682     2682              
- Misses         8262     8272      +10     
Flag Coverage Δ
javascript 15.41% <0.00%> (-0.02%) ⬇️
php 61.97% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hamza221 hamza221 force-pushed the Migrate/to-nextcloud-vue-8 branch from 4301ac0 to 23e3320 Compare December 29, 2023 13:53
@hamza221
Copy link
Contributor

pushed a commit here by mistake hence the push -f
NcSelect fix here with this branch as base #5649

@ChristophWurst
Copy link
Member

What is left to do here?

@GVodyanov GVodyanov requested a review from GretaD January 15, 2024 16:30
@GVodyanov GVodyanov added the 3. to review Waiting for reviews label Jan 22, 2024
@GVodyanov GVodyanov marked this pull request as ready for review January 22, 2024 17:39
@GVodyanov
Copy link
Contributor Author

GVodyanov commented Jan 25, 2024

Other issue found by Christoph, deleted calendars aren't striked through, will work on. Edit: solved

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

👍 tested & works

My only remark would be the NcSelects that have dynamic width. This makes the UI look jittery.

Bildschirmfoto vom 2024-01-29 09-56-18
Bildschirmfoto vom 2024-01-29 09-56-05
Bildschirmfoto vom 2024-01-29 09-55-47

NcSelect also seems to increase it's height when it is focused/open. Upstream bug or our styling?

@ChristophWurst ChristophWurst requested a review from st3iny January 29, 2024 11:24
@ChristophWurst
Copy link
Member

ChristophWurst commented Jan 30, 2024

  • BUG when I'm searching for attendees
[Vue warn]: Error in render: "TypeError: option is undefined"

found in

---> <VueSelect>
       <NcSelect>
         <InviteesListSearch> at src/components/Editor/Invitees/InviteesListSearch.vue
           <InviteesList> at src/components/Editor/Invitees/InviteesList.vue
             <NcAppSidebarTab>
               <NcAppSidebarTabs>
                 <NcAppSidebar>
                   <EditSidebar> at src/views/EditSidebar.vue
                     <NcContent>
                       <Calendar> at src/views/Calendar.vue
                         <App> at src/App.vue
                           <Root> vue.runtime.esm.js:4609

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

I tested a bit and couldn't spot any additional breakage.

I pushed some commits to fix multiple minor issues in the sidebar editor:

  • Vertical alignment of properties
  • Select widths overflowing on small screens
  • Hide the title submit button again

@GVodyanov
Copy link
Contributor Author

  • BUG when I'm searching for attendees
[Vue warn]: Error in render: "TypeError: option is undefined"

found in

---> <VueSelect>
       <NcSelect>
         <InviteesListSearch> at src/components/Editor/Invitees/InviteesListSearch.vue
           <InviteesList> at src/components/Editor/Invitees/InviteesList.vue
             <NcAppSidebarTab>
               <NcAppSidebarTabs>
                 <NcAppSidebar>
                   <EditSidebar> at src/views/EditSidebar.vue
                     <NcContent>
                       <Calendar> at src/views/Calendar.vue
                         <App> at src/App.vue
                           <Root> vue.runtime.esm.js:4609

Fixed

@ChristophWurst
Copy link
Member

ChristophWurst commented Feb 1, 2024

  • BUG room type is unusable

image

[Vue warn]: Error in render: "TypeError: option is undefined"

found in

---> <VueSelect>
       <NcSelect>
         <ResourceRoomType> at src/components/Editor/Resources/ResourceRoomType.vue
           <ResourceListSearch> at src/components/Editor/Resources/ResourceListSearch.vue
             <ResourceList> at src/components/Editor/Resources/ResourceList.vue
               <NcAppSidebarTab>
                 <NcAppSidebarTabs>
                   <NcAppSidebar>
                     <EditSidebar> at src/views/EditSidebar.vue
                       <NcContent>
                         <Calendar> at src/views/Calendar.vue
                           <App> at src/App.vue
                             <Root> vue.runtime.esm.js:4609
TypeError: option is undefined
    fn ResourceRoomType.vue:25
    VueJS 2
    fn NcSelect.mjs:430
    VueJS 2
    node_modules calendar-main.js:2269
    renderList VueJS
    node_modules calendar-main.js:2269
    VueJS 14
    onSearchFocus vue-select.js:1
    VueJS 33
vue.runtime.esm.js:3049

@ChristophWurst
Copy link
Member

ChristophWurst commented Feb 1, 2024

  • I still get a build warning
WARNING in ./src/components/AppNavigation/Settings/SettingsAttachmentsFolder.vue?vue&type=script&lang=js (./node_modules/babel-loader/lib/index.js!./node_modules/vue-loader/lib/index.js??vue-loader-options!./src/components/AppNavigation/Settings/SettingsAttachmentsFolder.vue?vue&type=script&lang=js) 8:4-14
export 'NcInputField' (imported as 'InputField') was not found in '@nextcloud/vue' (possible exports: Focus, Linkify, MOBILE_BREAKPOINT, NcActionButton, NcActionButtonGroup, NcActionCaption, NcActionCheckbox, NcActionInput, NcActionLink, NcActionRadio, NcActionRouter, NcActionSeparator, NcActionText, NcActionTextEditable, NcActions, NcAppContent, NcAppContentDetails, NcAppContentList, NcAppNavigation, NcAppNavigationCaption, NcAppNavigationIconBullet, NcAppNavigationItem, NcAppNavigationNew, NcAppNavigationNewItem, NcAppNavigationSettings, NcAppNavigationSpacer, NcAppSettingsDialog, NcAppSettingsSection, NcAppSidebar, NcAppSidebarTab, NcAutoCompleteResult, NcAvatar, NcBreadcrumb, NcBreadcrumbs, NcButton, NcCheckboxRadioSwitch, NcColorPicker, NcContent, NcCounterBubble, NcDashboardWidget, NcDashboardWidgetItem, NcDateTime, NcDateTimePicker, NcDateTimePickerNative, NcDialog, NcDialogButton, NcEmojiPicker, NcEmptyContent, NcGuestContent, NcHeaderMenu, NcHighlight, NcIconSvgWrapper, NcListItem, NcListItemIcon, NcLoadingIcon, NcMentionBubble, NcModal, NcNoteCard, NcPasswordField, NcPopover, NcProgressBar, NcRelatedResourcesPanel, NcRichContenteditable, NcRichText, NcSavingIndicatorIcon, NcSelect, NcSelectTags, NcSettingsInputText, NcSettingsSection, NcSettingsSelectGroup, NcTextArea, NcTextField, NcTimezonePicker, NcUserBubble, NextcloudVuePlugin, Tooltip, clickOutsideOptions, emojiAddRecent, emojiSearch, isA11yActivation, isFullscreen, isFullscreenState, isMobile, isMobileState, richEditor, useIsFullscreen, useIsMobile, userStatus, usernameToColor)
 @ ./src/components/AppNavigation/Settings/SettingsAttachmentsFolder.vue?vue&type=script&lang=js 1:0-196 1:212-215 1:217-410 1:217-410
 @ ./src/components/AppNavigation/Settings/SettingsAttachmentsFolder.vue 2:0-76 3:0-71 3:0-71 9:2-8
 @ ./node_modules/babel-loader/lib/index.js!./node_modules/vue-loader/lib/index.js??vue-loader-options!./src/components/AppNavigation/Settings.vue?vue&type=script&lang=js 8:0-81 27:4-29
 @ ./src/components/AppNavigation/Settings.vue?vue&type=script&lang=js 1:0-173 1:189-192 1:194-364 1:194-364
 @ ./src/components/AppNavigation/Settings.vue 2:0-59 3:0-54 3:0-54 9:2-8
 @ ./node_modules/babel-loader/lib/index.js!./node_modules/vue-loader/lib/index.js??vue-loader-options!./src/views/Calendar.vue?vue&type=script&lang=js 5:0-64 34:4-12
 @ ./src/views/Calendar.vue?vue&type=script&lang=js 1:0-167 1:183-186 1:188-352 1:188-352
 @ ./src/views/Calendar.vue 2:0-59 3:0-54 3:0-54 9:2-8
 @ ./src/router.js 26:0-44 41:15-23 54:15-23 104:15-23
 @ ./src/main.js 30:0-33 60:12-18 67:19-25 71:2-8

@st3iny
Copy link
Member

st3iny commented Feb 1, 2024

I still get a build warning

WARNING in ./src/components/AppNavigation/Settings/SettingsAttachmentsFolder.vue?vue&type=script&lang=js (./node_modules/babel-loader/lib/index.js!./node_modules/vue-loader/lib/index.js??vue-loader-options!./src/components/AppNavigation/Settings/SettingsAttachmentsFolder.vue?vue&type=script&lang=js) 8:4-14
export 'NcInputField' (imported as 'InputField') was not found in '@nextcloud/vue' (possible exports: Focus, Linkify, MOBILE_BREAKPOINT, NcActionButton, NcActionButtonGroup, NcActionCaption, NcActionCheckbox, NcActionInput, NcActionLink, NcActionRadio, NcActionRouter, NcActionSeparator, NcActionText, NcActionTextEditable, NcActions, NcAppContent, NcAppContentDetails, NcAppContentList, NcAppNavigation, NcAppNavigationCaption, NcAppNavigationIconBullet, NcAppNavigationItem, NcAppNavigationNew, NcAppNavigationNewItem, NcAppNavigationSettings, NcAppNavigationSpacer, NcAppSettingsDialog, NcAppSettingsSection, NcAppSidebar, NcAppSidebarTab, NcAutoCompleteResult, NcAvatar, NcBreadcrumb, NcBreadcrumbs, NcButton, NcCheckboxRadioSwitch, NcColorPicker, NcContent, NcCounterBubble, NcDashboardWidget, NcDashboardWidgetItem, NcDateTime, NcDateTimePicker, NcDateTimePickerNative, NcDialog, NcDialogButton, NcEmojiPicker, NcEmptyContent, NcGuestContent, NcHeaderMenu, NcHighlight, NcIconSvgWrapper, NcListItem, NcListItemIcon, NcLoadingIcon, NcMentionBubble, NcModal, NcNoteCard, NcPasswordField, NcPopover, NcProgressBar, NcRelatedResourcesPanel, NcRichContenteditable, NcRichText, NcSavingIndicatorIcon, NcSelect, NcSelectTags, NcSettingsInputText, NcSettingsSection, NcSettingsSelectGroup, NcTextArea, NcTextField, NcTimezonePicker, NcUserBubble, NextcloudVuePlugin, Tooltip, clickOutsideOptions, emojiAddRecent, emojiSearch, isA11yActivation, isFullscreen, isFullscreenState, isMobile, isMobileState, richEditor, useIsFullscreen, useIsMobile, userStatus, usernameToColor)

Ahh, this one. The component has to be imported directly from nextcloud/Vue/dist/Components/NcInputField.vue or something similar.

It is not included in the main bundle as we are not supposed to use it directly.

@GVodyanov
Copy link
Contributor Author

nextcloud/Vue/dist/Components/NcInputField.vue

Could you please specify exactly how to import it and where? I'm a bit confused

@ChristophWurst
Copy link
Member

ChristophWurst commented Feb 6, 2024

  • BUG: recurrence end selector

Bildschirmfoto vom 2024-02-06 12-21-11
Bildschirmfoto vom 2024-02-06 12-21-09

@ChristophWurst
Copy link
Member

ChristophWurst commented Feb 6, 2024

  • BUG (minor): Clear selection shows for the calendar picker that always has a value

Bildschirmfoto vom 2024-02-06 12-21-50

@GVodyanov
Copy link
Contributor Author

  • BUG: recurrence end selector

Bildschirmfoto vom 2024-02-06 12-21-11 Bildschirmfoto vom 2024-02-06 12-21-09

Leaving this comment here for future reference as this is a bit weird and it took me a while to understand: for some reason there is an internal css variable of vue-select called --vs-dropdown-min-width and it is set to 160px. Attempting to set an NcSelect to anything thinner than that will result in this bug, and it's hard to trace as this is internal styling of vue-select.

@GVodyanov
Copy link
Contributor Author

GVodyanov commented Feb 7, 2024

  • BUG (minor): Clear selection shows for the calendar picker that always has a value

Bildschirmfoto vom 2024-02-06 12-21-50

I'm a little blocked here, an input event is sent out with a value of an empty array but the value of the NcSelect doesn't change, could someone else take a look at this please?

EDIT: Solved by removing them :)

@GVodyanov
Copy link
Contributor Author

GVodyanov commented Feb 8, 2024

PropertySelectMultiple isn't working as intended, options aren't being removed from the dropdown when selected and an error is thrown, currently working on that.

EDIT: fixed

@GVodyanov
Copy link
Contributor Author

GVodyanov commented Feb 12, 2024

Other NcSelect bugs

  • There is no way to remove categories in PropertySelectMultiple
    • Only way to solve this is to make a custom close icon because the default one doesn't appear seeing as we embed a custom component
  • You can add the same category more than one time

EDIT: solved

@ChristophWurst
Copy link
Member

ChristophWurst commented Feb 19, 2024

  • BUG: alignment of appointments section and + icon
v4.5 Here
Bildschirmfoto vom 2024-02-19 13-10-25 Bildschirmfoto vom 2024-02-19 13-10-18

Richard: I can't reproduce this one.

@ChristophWurst
Copy link
Member

ChristophWurst commented Feb 19, 2024

  • BUG: calendars with spaces (e.g. shared calendars) are rendered as two lines once picked in the calendar picker
Selecting Selected
Bildschirmfoto vom 2024-02-19 13-12-21 Bildschirmfoto vom 2024-02-19 13-12-25

Ignore the broken avatar. That is an issue with my dev env.

EDIT: Is fixed now.
grafik

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

👍 otherwise. tested and works :)

Copy link
Member

@st3iny st3iny left a comment

Choose a reason for hiding this comment

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

Tested and works. Didn't have a look at the code.

@GVodyanov
Copy link
Contributor Author

  • BUG: alignment of appointments section and + icon

v4.5 Here
Bildschirmfoto vom 2024-02-19 13-10-25 Bildschirmfoto vom 2024-02-19 13-10-18

Richard: I can't reproduce this one.

Could you please suggest a way to reproduce? I can't get it to show either. @ChristophWurst

@ChristophWurst
Copy link
Member

Could you please suggest a way to reproduce? I can't get it to show either. @ChristophWurst

Checkout nextcloud/server#43640 🙈 So the change is not caused by your pr but the server styling update.

@st3iny
Copy link
Member

st3iny commented Feb 19, 2024

Well then time to resolve conflicts and squash commits @GVodyanov. Let me know if you need help with the conflicts.

@GVodyanov GVodyanov force-pushed the Migrate/to-nextcloud-vue-8 branch 2 times, most recently from ee615dd to 2c817aa Compare February 19, 2024 16:32
@st3iny st3iny force-pushed the Migrate/to-nextcloud-vue-8 branch from 2c817aa to 188e0cc Compare February 19, 2024 17:26
Signed-off-by: Grigory V <[email protected]>
Signed-off-by: Richard Steinmetz <[email protected]>
@st3iny st3iny force-pushed the Migrate/to-nextcloud-vue-8 branch from 188e0cc to fd355c9 Compare February 19, 2024 17:43
@st3iny st3iny enabled auto-merge February 19, 2024 17:48
@st3iny st3iny added technical debt 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 19, 2024
@st3iny st3iny merged commit 0895475 into main Feb 19, 2024
38 of 40 checks passed
@st3iny st3iny deleted the Migrate/to-nextcloud-vue-8 branch February 19, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bump @nextcloud/vue@7 to @nextcloud/vue@8
4 participants