Add feat: make winamp's window order configurable; Prevent windows from moving arbitrarily #133
Conversation
…nitialWindowLayout`)
jeffpaul
requested review from
a team and
Sidsector9
and removed request for
peterwilsoncc and
a team
There was a problem hiding this comment.
Hi @curiousRay, thanks for your PR.
I left some comments based on a high-level review. We may have to reiterate on this PR.
| } | ||
| value={ currentPosEqu } | ||
| > | ||
| <ToggleGroupControlOption label={ __( 'Top', 'winamp-block' ) } value="1" /> |
There was a problem hiding this comment.
The text domain everywhere should be retro-winamp-block instead of winamp-block.
|
|
||
| //console.log("posEqu: "+posEqu); | ||
| //console.log("posList: "+posList); | ||
| //console.log("posMilkdrop: "+posMilkdrop); |
There was a problem hiding this comment.
Seems like these were added for testing purposes and don't think its necessary for production.
| @@ -23,4 +23,102 @@ export const milkdropOptions = { | |||
| }, | |||
| }; | |||
|
|
|||
| const block = document.querySelector( '.wp-block-tenup-winamp-block' ); | |||
|
|
|||
| var posEqu = block?.dataset.posequ || "1"; | |||
There was a problem hiding this comment.
var is discouraged. Use let or const
| <ToggleGroupControlOption label={ __( 'Top', 'winamp-block' ) } value="1" /> | ||
| <ToggleGroupControlOption label={ __( 'Middle', 'winamp-block' ) } value="2" /> | ||
| <ToggleGroupControlOption label={ __( 'Bottom', 'winamp-block' ) } value="3" /> | ||
| <ToggleGroupControlOption label={ __( 'Hide', 'winamp-block' ) } value="0" /> |
There was a problem hiding this comment.
The value attributes should have meaningful values such as middle, hide etc instead of numbers so that we it is easily readable.
| @@ -0,0 +1,30 @@ | |||
| /** | |||
There was a problem hiding this comment.
We don't push asset build files such as .css, .min.js or .map files as these are built before deployment.
|
@curiousRay the player window should be draggable on the page, at least on non-mobile screens. |
Description of the Change
Closes #10 (#10 (comment))
Closes #83
Added configuration items for winamp block to set the windows' displaying order.
The Wordpress blocks can be placed into post/page, as well as sidebar widget. I've set rules separately. If you have any suggestions about these rules, just feel free to leave comments.
winamp block in post/page
winamp block in sidebar widget
How to test the Change
Note
Known issue before this PR: If two or more blocks appear in the same webpage, only one winamp instance will be created.
Changelog Entry
Credits
Props @dejalavidavolar
Checklist: