-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Migrate version tab to vue #26675
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 version tab to vue #26675
Conversation
| import './css/versions.css' | ||
| import './versionmodel'; | ||
| import './versioncollection'; | ||
| import './versionstabview'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this since you're now providing your own tab.
But for the development phase, you can leave it so you will have 2 versions tab and you will be more at ease when comparing the two :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ll leave it for the development phase
| --> | ||
|
|
||
| <template> | ||
| <li class="version-entry"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use a ListItemIcon component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| this.version.fullPath = fullPath | ||
| this.version.fileId = fileId | ||
| this.version.name = name | ||
| this.version.timestamp = parseInt(moment(new Date(version.timestamp)).format('X'), 10) | ||
| this.version.id = OC.basename(version.href) | ||
| this.version.size = parseInt(version.size, 10) | ||
| this.version.user = user | ||
| this.version.client = client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this.version? It does not seems to be declared anyhere.
I assume there is some code missing or this is still a WIP? 🤔 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed and updated in FileVersion Service
| const fetchVersion = await axios.get(shareUrl, { | ||
| params: { | ||
| format, | ||
| path, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you not using the davClient service(s)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileVersion Service - 6368672
|
|
||
| <template> | ||
| <ListItemIcon class="version-entry" | ||
| icon="icon-text" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The icon needs to be the mime type.
You can get it with the method: OC.MimeType.getIconUrl(mimetype).
It will return an url of the icon (to be used like the Avatar component: https://github.com/nextcloud/nextcloud-vue/blob/8d1dca2df1e7ee02b50b8b52355f801e6799220a/src/components/ListItemIcon/ListItemIcon.vue#L65)
e.g. OC.MimeType.getIconUrl('image/jpg')
| {{ version.timestamp }}Restore | ||
| </ActionButton> | ||
| <ActionButton icon="icon-delete" @click="alert('Delete')"> | ||
| Download |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs translation
| subtitle="< 1KB"> | ||
| <Actions> | ||
| <ActionButton icon="icon-edit" @click="alert('Edit')"> | ||
| {{ version.timestamp }}Restore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs translation
| if (OCA.Files && OCA.Files.Sidebar) { | ||
| OCA.Files.Sidebar.registerTab(new OCA.Files.Sidebar.Tab({ | ||
| id: 'versions', | ||
| name: t('files_versions', 'version'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| details: true, | ||
| }, options)) | ||
|
|
||
| return response.data.map(FileVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FileVersion doesn't exists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import Videos from '../models/videos' | ||
| import Audios from '../models/audios' | ||
|
|
||
| export default class Viewer { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its removed, it was just for reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| fileInfo: null, | ||
| currentUser: null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed and updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| setCurrentUser(user) { | ||
| this._currentUser = user | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed and updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <IfModule pagespeed_module> | ||
| ModPagespeed Off | ||
| </IfModule> | ||
| #### DO NOT CHANGE ANYTHING ABOVE THIS LINE #### |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change come from your development setup with a specific configuration, so please do not commit that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok updated
| const path = (this.fileInfo.path + '/' + this.fileInfo.name).replace('//', '/') | ||
| // Fetch Version | ||
| const fetchVersion = await axios.get(shareUrl, { | ||
| const fetchFileVersions = await axios.get(shareUrl, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes removed and Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| import { translate as t, translatePlural as n } from '@nextcloud/l10n' | ||
|
|
||
| import VersionTab from '../../files_versions/src/views/VersionTab' | ||
| import TabSections from '../../files_versions/src/services/TabSections' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would this be about? There is not tab section service yet ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| console.log(error); | ||
| } | ||
| } | ||
| mounted() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mounted is not expected to be within the methods but at the root of the vue component object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok it updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, missing comma on 114
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>

Ref #20020