-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[RNMobile] Audio block URL Parser unit tests #31920
Merged
jd-alexander
merged 60 commits into
trunk
from
rnmobile/audio-block-IV-from-url-unit-tests
May 21, 2021
Merged
Changes from all commits
Commits
Show all changes
60 commits
Select commit
Hold shift + click to select a range
0519567
Basics of Audio block working
etoledom 690224d
Add audio support to MediaUpload
etoledom b7965fd
Add handling of file uploads and replace
etoledom f2ccb18
WPMediaLibrary support for Audio block
etoledom 2a02b83
Avoid removing media info on error state
etoledom 276b07d
Linting
etoledom 8a8b550
Added an AUDIO file to the test requestMediaPickFromMediaLibrary func
jd-alexander 9e03e63
Fixed typo in ToolbarButton of Audio Block.
jd-alexander 349aa19
Removed auto help behavior present on web that's not used on mobile.
jd-alexander 04a39b4
Merge branch 'master' into rnmobile/audio-block-I-b
jd-alexander 32029e0
[Android] Wired the click of the Audio Media Library button.
jd-alexander a18b8ac
Added Audio media options for choosing audio file locally.
jd-alexander af6cb99
[RNMobile] Audio Block: Proper caption field (#27689)
ceyhun 193339f
Add alert prompt to insert from URL
ceyhun 4ab2988
Set entered URL in audio block attributes
ceyhun d9a6041
Merge branch 'master' into rnmobile/audio-block-I-b
jd-alexander 1a46994
Merge branch 'master' into rnmobile/audio-block-I-b
jd-alexander f09c955
Merge branch 'master' into rnmobile/audio-block-I-b
jd-alexander fff09be
initial integration with react native prompt.
jd-alexander d4209e8
updated prompt lib.
jd-alexander 9d61d49
Merge branch 'rnmobile/audio-block-I-b' into rnmobile/audio-block-IV-…
jd-alexander f0a1034
updated commit hash of prompt lib.
jd-alexander 1eb58d2
Updated package-lock.json.
jd-alexander 17dea27
Added RNPromptPackage to Glue Code.
jd-alexander c7154d3
Merge branch 'master' into rnmobile/audio-block-IV-from-url
jd-alexander 0f597f3
fixed merge conflicts between the edit.native.js files of the branches
jd-alexander e4dec03
added onSelectURL to the media-upload component
jd-alexander f77a861
added onSelectURL to the media-placeholder
jd-alexander cad8e8a
updated onSelectURL support with notice and URL validation
jd-alexander ef478bb
sync package-lock.json
jd-alexander ee9a536
Merge remote-tracking branch 'origin/master' into rnmobile/audio-bloc…
jd-alexander 6b6221f
updated package-lock.json
jd-alexander 377ceae
updated package-lock.json
jd-alexander 01242f5
Merge branch 'trunk' into rnmobile/audio-block-IV-from-url
jd-alexander 5911b09
updated react-native-prompt-android setup due to binary dep changes.
jd-alexander cebdf5f
sync package-lock.json
jd-alexander 2dd3b6c
Integrated RNPromptPackage with the glue code.
jd-alexander f348df3
sync package-lock.json
jd-alexander b81dd77
package-lock.json fix
jd-alexander d4ad088
sync package-lock.json
jd-alexander 754b999
Added the globe icon to the Insert from URL source.Only shown on Android
jd-alexander e7ab21b
Extract filename from all URL types and optimized extension logic.
jd-alexander 1834b58
Merge branch 'trunk' into rnmobile/audio-block-IV-from-url
jd-alexander 614d57b
Merge branch 'trunk' into rnmobile/audio-block-IV-from-url
jd-alexander 6170600
Merge branch 'trunk' into rnmobile/audio-block-IV-from-url
jd-alexander b864661
Extracted the url parsing logic to a file so it can be tested.
jd-alexander 875441c
integrated parseAudioUrl function with the Audio block.
jd-alexander 8d2d545
Created tests for the audio url parsing.
jd-alexander d79719f
Made the component and its respective tests native.
jd-alexander 1e550e6
Merge branch 'trunk' into rnmobile/audio-block-IV-from-url
jd-alexander e68550e
Reduced onPress -> onSelectURL logic since the parameters match.
jd-alexander 024ecad
Added entry to CHANGELOG.md
jd-alexander 7fd4385
Merge branch 'rnmobile/audio-block-IV-from-url' into rnmobile/audio-b…
jd-alexander 462fd4f
Utilized getPath from the `wordpress/url` package to simplify logic.
jd-alexander 05a8335
Added URL with folder to the tests for audio urls with an extension.
jd-alexander e4ea54b
Enhanced tests to verify the title and extension values.
jd-alexander 15bd47e
Trimmed the extension since a space exists in the string for the UI.
jd-alexander 92752b2
Removed trim and put the space in the string.
jd-alexander f33dd98
Merge branch 'trunk' into rnmobile/audio-block-IV-from-url-unit-tests
jd-alexander 2bbecc8
Removed duplicate entry.
jd-alexander File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
12 changes: 12 additions & 0 deletions
12
packages/components/src/mobile/audio-player/audio-url-parser.native.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { safeDecodeURI, getPath } from '@wordpress/url'; | ||
|
||
export const parseAudioUrl = ( src ) => { | ||
const fileName = getPath( safeDecodeURI( src ) ).split( '/' ).pop(); | ||
const parts = fileName.split( '.' ); | ||
const extension = parts.length === 2 ? parts.pop().toUpperCase() + ' ' : ''; | ||
const title = parts.join( '.' ); | ||
return { title, extension }; | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
38 changes: 38 additions & 0 deletions
38
packages/components/src/mobile/audio-player/test/audio-url-parser.native.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { parseAudioUrl } from '../audio-url-parser.native'; | ||
|
||
const supportedAudioUrlsWithExtensions = [ | ||
'https://www.mp3.com/file.mp3?key1=value1&key2=value2#anchorforsomereason', | ||
'https://www.mp3.com/file.mp3?key1=value1&key2=value2', | ||
'https://www.mp3.com/file.mp3', | ||
fluiddot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
`https://www.mp3.com/folder/file.mp3`, | ||
]; | ||
|
||
const supportedAudioUrlsWithoutExtensions = [ | ||
'https://www.mp3.com/file?key1=value1&key2=value2#anchorforsomereason', | ||
'https://www.mp3.com/file?key1=value1&key2=value2', | ||
'https://www.mp3.com/file', | ||
'https://www.mp3.com/folder/file', | ||
]; | ||
|
||
describe( 'supportedAudioUrlsWithExtensions', () => { | ||
supportedAudioUrlsWithExtensions.forEach( ( url ) => { | ||
it( `supports ${ url }`, () => { | ||
const { title, extension } = parseAudioUrl( url ); | ||
expect( title ).toBe( 'file' ); | ||
expect( extension ).toBe( 'MP3 ' ); | ||
} ); | ||
} ); | ||
} ); | ||
|
||
describe( 'supportedAudioUrlsWithoutExtensions', () => { | ||
supportedAudioUrlsWithoutExtensions.forEach( ( url ) => { | ||
it( `supports ${ url }`, () => { | ||
const { title, extension } = parseAudioUrl( url ); | ||
expect( title ).toBe( 'file' ); | ||
expect( extension ).toBe( '' ); | ||
} ); | ||
} ); | ||
} ); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back 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.
I see that if the file contains an extension. we're adding an extra blank space at the end of the string, what's the reason for this?
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.
Ah, yes, I decided to add a space here because there are times when the
extension
isn't present but if it is I need to add the space here so within the UI itself the extension doesn't get attached to theaudio file
Text in the UI. For more context, see this comment #27817 (comment)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.
Oh ok, I'm wondering if the extra space should be handled then in the component instead since it's specific to how to render it, wdyt? My first impression was that this method was agnostic to the component but maybe since it's located under the same folder makes sense to include it 🤔 .
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 are right about the component handling this especially since it's UI-related. I could create another small PR to resolve this since this PR solved the main need which was to add the tests 😄