Changed layout

[annoyingmouse]

Using CSSStyleSheet and a single template (for film and TV; couldn't get songs to work - shouldn't be too hard to add them again though). Removed duplicate CSS for the dark theme as well. I've created a new version with my changes, so you should be able to see what to change and where - I'm also a bit of a neat freak, so I've tidied up the demo page - you might want to remove the comments around the details elements...? I'm not handling errors and I did ponder adding a CSS skeleton for loading data, but I don't have the time today.

[MarketingPip]

@annoyingmouse - awesome job & glad to see someone else join in on this! In future different API's & theme's could be implemented - maybe even current playing etc.

In terms of CSS skeletons - I am brutal when it comes to responsiveness design ATM so I rather not touch it. But if you would like to add them for it, I think it would be a very nice feature / touch instead of just loading.

Keep in mind - either, me or you will have take care of error handling (as was done before - besides for the song value).

If you want to go ahead & do this, let me know or I can take care of this + adding the song card back in when I get a chance!

Appreciate your value added to this project :+1:

[annoyingmouse]

Fair enough, the responsive CSS is still there, it's just simplified (you'd duplicated all the CSS variables for the dark theme - I simply removed the extraneous CSS and copied across only the relevant stuff that didn't come from the original CSS.)

I might be able to dig in and help, depending on work commitments. The error handling should be easy enough to implement - as should the skeleton CSS for slow connections.

[MarketingPip]

Ah in terms of responsiveness for the skeleton CSS! But if you look at the previous version I had you will see how to handle errors, only thing that is not done if song JSON data is empty - a message needs shown which I did not add the fix for yet.

Again, just let me know if you plan on handling these soon / or the tasks you will handle soon enough, and I will do the rest from there (if needed)!

As well if you're game for another project, you might want an invite on another web component / API based web element (s) coming up. Again most thing's are done, but always room for improvements.

[MarketingPip]

@annoyingmouse - as well going through looking at your changes live there seems to actually be some css issues now

Notice the text length, colors & placement of minutes etc.

[annoyingmouse]

@MarketingPip if I have some time I'll address these points today.

[annoyingmouse]

Yep, sorry, when I changed the class names IO forgot the change them within the media queries! D'oh! Also forgot to remove all margins within the * 0 should be better now @MarketingPip :-)

[MarketingPip]

Yep, sorry, when I changed the class names IO forgot the change them within the media queries! D'oh! Also forgot to remove all margins within the * 0 should be better now @MarketingPip :-)

@annoyingmouse no worries! I fixed the background images not working correctly in your commits, as well console logged music results for you. Usage was same as before.

Tho we need to add CSS styles in again for this + we need to add error handling as was done before for this PR to be committed. (Invalid API key, no API key, movie not found, song not found etc.)

Something else that needs done for mobile (media query sizes), the top image needs the left: 33% removed in the CSS. To fix this issue below.

with left: 33% removed.

Again this needs done for mobile only! All the CSS for song card you can find in script in the folder 1.0.1 assuming you are wanting to take that on too keep things clean as you were before!

[MarketingPip]

@annoyingmouse - you reverted my changes for background image issue + CORS should be set when getting song data. Which there appears to be an issue with the changes you made pushed. Tho the loading skeleton looks nice!

[annoyingmouse]

@annoyingmouse - you reverted my changes for background image issue + CORS should be set when getting song data. Which there appears to be an issue with the changes you made pushed. Tho the loading skeleton looks nice!

Yeah, there were a fair few merge conflicts, sorry about that - if you make them again, then I'll hang fire and do a pull when they're up - I'm still unable to access the iTunes API - I'll see about moving to a different development environment at some point today.

[MarketingPip]

@annoyingmouse - you reverted my changes for background image issue + CORS should be set when getting song data. Which there appears to be an issue with the changes you made pushed. Tho the loading skeleton looks nice!

Yeah, there were a fair few merge conflicts, sorry about that - if you make them again, then I'll hang fire and do a pull when they're up - I'm still unable to access the iTunes API - I'll see about moving to a different development environment at some point today.

@annoyingmouse if you look at the second commit I pushed. You will see I console logged a song for you / all the JSON data you need.

Example usage

<media-element name="The Beatles In My Life" type="song"></media-element>

Try accessing / running my commits of the script via CodePen or JS fiddle

[annoyingmouse]

Hi @MarketingPip - I've cloned to Glitch (https://glitch.com/edit/#!/foremost-sly-monitor?path=index.html:98:4) and tried running it locally as well and I don't get any song details: https://foremost-sly-monitor.glitch.me Removing the .media-card before your initial CSS for blur-back means that we can unset the left does that make sense, it's all a matter specificity.

[MarketingPip]

Just to clarify - you are using / viewing this commit?

As I ONLY console logged the values for now needed for the song. Just so you can see where the data is. (We still need to add the card in + CSS needed)

If any issues with this I will post a JS Fiddle!

In terms of the left 33% issue - I was assuming we would just removing the left for media queries with mobile size - set it to 0 etc.

Tho if you have a better solution, please do implement it. As said too, we still need to deal with error handling as was done before.

[annoyingmouse]

Yeah, that's the issue - I'm not getting anything back from the fetch, it's being blocked, no matter what mode I set the CORS - my email address is annoyingmouse@gmail.com - lets try and sort out something online.

[MarketingPip]

Yeah, that's the issue - I'm not getting anything back from the fetch, it's being blocked, no matter what mode I set the CORS - my email address is annoyingmouse@gmail.com - lets try and sort out something online.

hmmm - don't know if something got messed up with our commits etc as it worked using this before

<media-element name="In My Life The Beatles" type="song"></media-element>

but try this now & you should see a response.

<media-element name="In My Life - The Beatles" type="song"></media-element>

I will have to look later what got messed up if you can not see wherever that issue came out of now. (Requires the song to be spelled wrong / something with the json results being empty with correct name)

I will add you via e-mail if needed, that being said prefer just keeping issues here for everyone to see etc. As well helps focus on work tbh.

[MarketingPip]

@annoyingmouse - went ahead and got things started for song cards. Some changes will need to be made still to reflect previous. Tho I noticed this web element tends to get stuck loading some elements.

I will have to ask another member in the community if they are willing to take a look being they have more experience in the JavaScript field.

Please look here - as I didn't not want to over-write your commits.

https://github.com/annoyingmouse/Media-Element.js/blob/main/review_me.js

As well if you want to see if you can figure out the issue with the need for hyphen in name now - that'd be cool too!

[annoyingmouse]

LOL, I've managed to mirror the iTunes API, I'm nearly there with songs now...

[MarketingPip]

@annoyingmouse

ps; if you want to go ahead and add a limit for genres Max (3) - so they aren't over top of image's that'd be great.

But looks like we are close to making this PR!

[annoyingmouse]

Have at it. I'll try to tweak things when I have time...

[MarketingPip]

@annoyingmouse - cleaned your recent pushes, fixed undefined genres (don't know if you were working on this) & finished the song card.

(Please make sure to review changes an accept PR on it)

The text is not 100% accurate as before, couldn't figure out the proper CSS or if I just haven't had enough coffee. lol

But it seems the song card is complete & we just need to add error values!

Dang we make a good team :hand:

[MarketingPip]

@annoyingmouse apologizes - GitHub would not allow me to make a pull request into that repo for whatever reason and I did not just mean to overwrite your recent file. Especially with no changes documented in the title.

Tho - as said, the song card is almost near 100% complete. We just need to implement error card issues next I do believe, add 3 values into array for genre (etc) & don't quote me but I think another class for song cards.

As well - don't know your CSS levels but I was not able to figure out a good fix for properly fitting text descriptions.

Mentioned to previously in the to-do, what are your thoughts on using another API provider for album cover art? Being iTunes only provides 100x100 pixel images. Meaning limited themes can be provided in the future without them looking cruddy.

[annoyingmouse]

Sorry, I'd already limited the genres to 3 in my last push. I don't mind where images are sourced. I don't think you've overwritten my files at all - I did see a commit, but not to my branch... Also - I'd suggest not being so specific with CSS - we're in the shadow DOM and if we get specific in one place, we need to be so in the CSS that overwrites it, much nicer to not be so...

[MarketingPip]

@annoyingmouse - just added your genres back in with my fixes.

Attaching some pictures of what needs done for photo reference.

• [X] Movie Genres

• [ ] TV Genres (Not done - was undefined with your commits)

• [X] Song Card

As for the font issue - I think I am literally just tired but just wanting a confirm the font weights are the same as previous version for song card. (Might be my eye's seeing in different resolution lol)

And reason I mentioned possibly another API provided for music as said if using 100 pixel images, we are limited to cards that can be designed. Hence right now the music card background is on repeat instead of seeing pixels like this

There is an album cover art provider that does not require an API key if we decide to go about this.

Updating the source in your repo again after this comment.

[MarketingPip]

@annoyingmouse please note for whatever reason - don't ask me why

it seems <media-element name="The Beatles In My Life" type="song"></media-element> works now but

<media-element name="The Beatles - In My Life" type="song"></media-element> does not.

[MarketingPip]

Mobile card / images has been fixed as far as I know. Some other max-width sizes might need applied but!

[MarketingPip]

@annoyingmouse some issues with error handling - if no API key message needs added + background position's for movies / tv show's are in-correct now (moved to far to left) - I can't see where the issue is as of right now tho. As well - we can not use width in percentage for the cards & will need to revert to original sizing (in px's). We will need to look at another fix for fitting text content, maybe even use a JS library to fit the content automatically to the div.

As well - we need to revert some changes. As well you need to watch the files you are over-writing, merging as they are merging / over-writing our work.

The loader needs worked on as it messes with the CSS. I tried figuring this out but I will push a new commit. Again please look over files before making commits, as we keep over-writing each other's work.

[annoyingmouse]

@MarketingPip - genres added to TV now, sorry - didn't clock that TV could have genres... errors are handled similarly to your original code.

[MarketingPip]

@MarketingPip - genres added to TV now, sorry - didn't clock that TV could have genres... errors are handled similarly to your original code.

@annoyingmouse assuming you seen my references to us over-writing each-other's code? I did some more error handling today but as said waiting to clear things up with you on your end so we don't do that again haha!

That said too - we need to revert CSS - but I was having issues when doing so using your recent commits.

As said we will have to go to px options and possible use a JS library etc. Or maybe get some more insight from someone else on how to deal with CSS / font-sizing.

ps: Example, your last commit does not have the max 3 genres etc.

[annoyingmouse]

I'm not sure if we need to revert the CSS - sort of why I was asking for a meeting, TBH - I'm quite happy with the way it looks at present, but I guess it's your baby...

[MarketingPip]

I'm not sure if we need to revert the CSS - sort of why I was asking for a meeting, TBH - I'm quite happy with the way it looks at present, but I guess it's your baby...

The movie cards are not correct size - and reason being we need to revert it is for responsiveness. As much as I understand your reasoning to auto-fit the content for now. As well - the background's are messed up due to the loading state. Which really should be removed (if details are fetched) / replaced if DOMcontent is not loading etc.

Don't mean to throw out your hard work as I did see all the changes you made in the css to make things fit. (my apologizes)

Tho if you try using The Matrix (1999) - you will see height issues, background image issues compared to original as well if you use Spongebob (TV show) you will notice the card details do not match previous version either. Text is to long now etc. Minutes are not correctly placed and more.

Edit: as well - not just my baby. This project will be communities baby at one point so don't feel afraid to voice inputs on it at all!

[MarketingPip]

@annoyingmouse - awesome job, could not figure out what I was doing wrong.

Few minor things before I make some changes & accept / merge this PR.

The background image (is still not 100%) correct. When applying a left:33% on the image - does not apply the correct look as before.

The genres (3 count) for the movie was missed / over-written again. And the TV genre's fetch point was over-written with movies.

If you want to commit these - changes or me to re-add them please let me know.

Last thing(s) before I merge this, what do you think about removing the width in percentage? I know you are wanting to keep it at percentage to fit the text - but again. Thinking we need to find another solution for this. (Again I am not strong in CSS - so if there is a reason you would advise NOT to. Please state it. )

And I will need to add some other error handling in - that I have finished just don't want to over-write each other's work etc. (No API key provided, if song type = if (total_results == 0) ) & and if any other I am forgetting please do let me know.

Hoping all is well & hoping you are having a great weekend.

Sorry as well for my dry tone / responses in these recent replies etc. Been a busy week in general here!

[annoyingmouse]

@MarketingPip - don't worry about your tone. Let me take a look at your points above and I'll try to fit some time in this weekend.

[MarketingPip]

@annoyingmouse haha well always hard depicting emotion in e-mail etc hence I try to throw in the odd emoji here in there. But I figured they might have been somewhat dry / bland responses being late or tired writing them haha! :smile:

But take your time - they are just small questions and the one I need your final input on as thinking you might have a reason to keep the width. I just figured it might mess with responsiveness issues.

[annoyingmouse]

Oh, the things I'll do while waiting for the gym to open; I think I've addressed your points now. Have a look. If not, let me know, and I'll have a squint later.

[annoyingmouse]

One thing to note, I've altered the way of displaying genres, so you can have more than 3 without it looking odd @MarketingPip

[MarketingPip]

Woo-hoo! Awesome job! Merging this now! I am going to add you to another repo to ask you a question so we don't blow this up! (As it is not related to this). Ps; I will be adding one small edit after this merge (if no poster image = display no poster image), feel more than welcome to change the image to something else & I will merge it again!

[MarketingPip]

One other thing that could used improved / reverted, (CSS fix) is reverting text to match left align like before for song.

Original for reference:

As well - sometimes the web element does not wait till fetch loaded and returns no content / errors.

[MarketingPip]

@annoyingmouse sorry for delay! Hoping all is well on your end!

I will try & add you to the other web component based repo's and see if you want to take a look at hopping in on those! As well was hoping you'd look over the small changes & tell me what you think.

Another small issue I seen was when using Spongebob (movie) the genre's + card is messed up due to to many genres.

Should we add the limit back in again?

[annoyingmouse]

Hi @MarketingPip - I saw some changes and made more on my end; I've not committed them as I don't want to raise another PR just yet (I'm busy with other stuff at the minute). Aye, happy to take a look, though you'll perhaps have already noticed that I'm somewhat opinionated about coding style ;-) If you're happy with that then yeah, let me at them.

[annoyingmouse]

In terms of the genres, I'm not sure we should limit them, we could display them differently though...?

[MarketingPip]

Too be honest I don't wanna mess with the CSS to much myself as I was struggling trying to get the text-aligned back to left.

Tho I can try and take a jab at resolving it if needed! Another thing I was just noticing is the file size of this (40kb) "un-minified". I don't know what else we can do beside's minify the CSS used in the script then re-minify the JS again to bring this down to improve performance?

As in terms of your code style - not at all bothered haha! More than happy - tbh I am a noob at JS & have no one really to learn from locally etc or really even online tbh. So it's helps me improve big time looking at things I should be doing properly with some real code style & proper / improved logic.

[MarketingPip]

@annoyingmouse - don't know if error on my end or what. But this version of web component is having a problem when I try putting in a certain div (again don't know if something on my end) but in the index page you will see I had to put an onload feature to add the one element in. Testing on JSfiddle works fine tho for whatever reason!

[annoyingmouse]

@MarketingPip - want to share the JSFiddle, and the code?

[MarketingPip]

@MarketingPip - want to share the JSFiddle, and the code?

The index file in this repo - as said on GitHub pages. Getting an error when keeping one element in div (returns random movie called "Null Null" lol) when trying to put 8 mile inside a div for demo page improvement.

Tho the same code on JS fiddle works, that being said I am not using the web element from script source but rather inside the JSfiddle JS section.

TLDR: for this element to properly show up on GitHub pages I had to use a onload feature to add it to the HTML document. Otherwise it returned "Null Null" movie.

As said too - sometimes the web element loads movies / tv shows - sometimes is does not. I experienced this most times when using the demo file before.