-
Notifications
You must be signed in to change notification settings - Fork 38
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
Ms-file-viewer-bottombar #8965
Ms-file-viewer-bottombar #8965
Conversation
handler: () => void; | ||
} | ||
|
||
defineProps({ |
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.
defineProps<{
actions: Array<FileViewerAction>;
}>();
should work, why the extra steps?
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.
Done
client/src/parsec/mock_generator.ts
Outdated
@@ -117,7 +117,7 @@ export async function generateFolder( | |||
export async function generateEntries( | |||
basePath: FsPath, | |||
folderCount = 2, | |||
fileCount = 2, | |||
fileCount = 10, |
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.
Don't forget to revert this later.
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.
Done
const src = ref(''); | ||
const audioElement = ref(); | ||
const paused = ref(true); |
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.
Do you need this? You can access the paused
value on the element itself to get its state. Same with volume.
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 we need this part: audioElement as ref don't trigger its "change" state when using media controls.
import { ref, Ref, type Component, inject, onMounted, shallowRef } from 'vue'; | ||
// eslint-disable-next-line max-len |
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.
Nop, use prettier to format the 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.
done
import { MsActionBar, MsActionBarButton, Translatable } from 'megashark-lib'; | ||
import { PropType } from 'vue'; | ||
|
||
interface FileViewerAction { |
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'll probably want a disabled
attribute to prevent clicking in some cases (max zoom level reached for example).
Also better to extract this interface to a separate file to be able to import it for typing.
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.
Since the action bar will change depending on the feature list we will make with Fabien on monday, I just add your suggestions as comment to keep it in mind in the next PR.
0a2164a
to
33b6167
Compare
- Create a FileViewerWrapper component to wrap the FileViewer and its action bar - Isolate the FileViewerActionBar as a common component - Update all the viewers component to use the new structure - Improve all viewers UI/UX - Add translation for 'opening file'
33b6167
to
24e4998
Compare
No description provided.