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

Rewrite the settings in vue #589

Closed
wants to merge 2 commits into from
Closed

Rewrite the settings in vue #589

wants to merge 2 commits into from

Conversation

artonge
Copy link
Collaborator

@artonge artonge commented May 19, 2021

This is a rewrite of the settings in Vue.js

  • Switch to initial state
  • Convert templates to Vue
  • Create a store for the settings
  • Add Cypress tests
  • Delete legacy files

User settings

Before After
Screenshot 2021-07-01 at 15-16-18 Settings - Nextcloud Screenshot 2021-07-01 at 15-19-10 Settings - Nextcloud

Admin settings

Before After
Screenshot 2021-07-01 at 15-16-29 Settings - Nextcloud Screenshot 2021-07-01 at 15-19-20 Settings - Nextcloud

@artonge artonge force-pushed the feature/vue_rewrite branch 2 times, most recently from a11016d to 5247f56 Compare June 28, 2021 10:32
@artonge artonge self-assigned this Jun 28, 2021
@artonge artonge force-pushed the feature/vue_rewrite branch 3 times, most recently from b3eaa8d to 848875e Compare June 30, 2021 17:21
@@ -8,5 +8,6 @@ module.exports = {
'node/no-unpublished-import': ['error', {
allowModules: ['@vue/test-utils', '@testing-library/vue'],
}],
'valid-jsdoc': 'off',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Needed until a new version of nextcloud/eslint-config is released


<template>
<div>
<!-- TODO: should I use the global grid class ? -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@skjnldsv ? An opinion ?

/**
* Toggle the availability of mail notifications
*
* @param {import('vuex').ActionContext<SettingsState, SettingsState>} store -
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ugly way to add typing from a node_module

@artonge artonge marked this pull request as ready for review July 1, 2021 13:43
@artonge artonge requested review from nickvergessen, skjnldsv and icewind1991 and removed request for nickvergessen July 1, 2021 13:44
@artonge
Copy link
Collaborator Author

artonge commented Jul 1, 2021

@skjnldsv the background around an hovered checkbox is not round, is it on purpose ?

Screenshot from 2021-06-30 19-14-37

Copy link
Member

@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.

Looks good but didnt test

src/models/NotificationsFrequency.js Outdated Show resolved Hide resolved
@artonge artonge force-pushed the feature/vue_rewrite branch 2 times, most recently from e6465b0 to d75bc18 Compare July 1, 2021 16:22
@artonge artonge force-pushed the feature/vue_rewrite branch 2 times, most recently from fa62f01 to 8cbc10a Compare July 6, 2021 16:03
Signed-off-by: Louis Chemineau <[email protected]>
Signed-off-by: Louis Chemineau <[email protected]>
@szaimen szaimen added this to the Nextcloud 23 milestone Aug 11, 2021
@AndyScherzinger

This comment has been minimized.

@CarlSchwan
Copy link
Member

This was integrated in another PR

@CarlSchwan CarlSchwan closed this Aug 30, 2022
@nickvergessen nickvergessen deleted the feature/vue_rewrite branch August 30, 2022 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants