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

Use new app settings components #4195

Merged
merged 1 commit into from
Nov 12, 2020
Merged

Use new app settings components #4195

merged 1 commit into from
Nov 12, 2020

Conversation

marcoambrosini
Copy link
Member

@marcoambrosini marcoambrosini commented Sep 22, 2020

@nickvergessen nickvergessen added this to the 💚 Next Major (21) milestone Oct 5, 2020
@marcoambrosini marcoambrosini force-pushed the appsettingssection branch 3 times, most recently from a06124f to 67fa2c4 Compare October 19, 2020 09:59
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

This error appears now in the browser console when opening the main Talk UI (even if the settings dialog is not opened):

[Vue warn]: Error in updated hook: "TypeError: this.$el.querySelector is not a function"

found in

---> <AppSettingsDialog>
       <SettingsDialog> at src/components/SettingsDialog/SettingsDialog.vue
         <Content>
           <App> at src/App.vue
             <Root>

Additionally, clicking on the headers in the left area causes the right area to scroll to the appropriate section, but scrolling in the right area does not change the highlighted header in the left area (although this may be an issue of the component from nextcloud-vue, I do not know).

I have also noticed that content in the right area can not be selected using the mouse (not that I want to, but I found it strange :-P ) and, in Firefox, it is not possible to scroll in the right area using the arrow keys (although it works in Chromium). However it also happened before using the new AppSettings components, so it could be a problem with the Modal component (just a guess). Opened issues in nextcloud-vue.

Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

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

Tested with nextcloud-libraries/nextcloud-vue#1503, but there is still something strange with the blurring (not sure if the issue is in the component or in how it is used):

  • Open the settings
  • Close the settings
  • Open the settings
  • Click on the second header
  • Scroll

The header will not be blurred.

@PVince81
Copy link
Member

Missing padding on top:
image

@PVince81
Copy link
Member

seems this was closed by mistake

@PVince81 PVince81 reopened this Oct 28, 2020
@PVince81
Copy link
Member

PVince81 commented Nov 2, 2020

please note that nextcloud-vue was updated, so many stuff that wasn't working before is working now.
you might need to rebase

@marcoambrosini
Copy link
Member Author

Ready for review!

@PVince81
Copy link
Member

PVince81 commented Nov 5, 2020

a wild conflict appears!

Signed-off-by: Marco Ambrosini <marcoambrosini@pm.me>
@marcoambrosini
Copy link
Member Author

Rebased!!

@nickvergessen nickvergessen merged commit a26d1bc into master Nov 12, 2020
@nickvergessen nickvergessen deleted the appsettingssection branch November 12, 2020 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants