-
Notifications
You must be signed in to change notification settings - Fork 373
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
[ui-storagebrowser] fixes the preview for non-text files #3981
base: master
Are you sure you want to change the base?
Conversation
@@ -295,18 +305,18 @@ describe('StorageFilePage', () => { | |||
expect(video.children[0]).toHaveAttribute('src', expect.stringContaining('videofile.mp4')); | |||
}); | |||
|
|||
it('should display a message for unsupported file types', () => { | |||
it('should display a message for compresed file types', () => { |
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.
nit: spelling
@@ -230,15 +230,25 @@ describe('StorageFilePage', () => { | |||
expect(screen.queryByRole('link', { name: 'Download' })).toBeNull(); | |||
}); | |||
|
|||
// TODO: fix this test when mocking of useLoadData onSuccess callback is mproperly mocked | |||
it.skip('should render a textarea for text files', () => { | |||
// TODO: fix this test when mocking of useLoadData onSuccess callback is properly mocked |
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.
Why don't we mock this now?
const fileExtension = fileName?.split('.')?.pop()?.toLowerCase(); | ||
if (!fileExtension) { | ||
return SupportedFileTypes.OTHER; | ||
for (const fileExtension in SUPPORTED_FILE_EXTENSIONS) { |
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.
Why did you remove the split on "." lower case transformation? This seems less robust given that this function still receives the complete file name.
A file called "testpng" and that is lacking a proper file type will now be assumed to be a png file just because it has it in the name at the end. Also "test.PNG" which should be matched won't be matched.
What changes were proposed in this pull request?
Dockerfile
) was opened in the file browser, a message indicating "unsupported file type" was shown, which prevented users from viewing the content of the file, even if it contained readable text.How was this patch tested?
Please review Hue Contributing Guide before opening a pull request.