codefordenver / Comrad

Open-source web application for radio stations to manage show schedules, traffic and compliance
ISC License
25 stars 9 forks source link

Add Loading component to Show Builder page #865

Closed kmid5280 closed 11 months ago

kmid5280 commented 11 months ago

Guidelines

DEVELOPMENT branch on the LEFT
YOUR branch on the RIGHT

Issue

This issue resolves #831

Description

Imported Loading component and created conditional to display it when shows are being fetched.

seankwilliams commented 11 months ago

@kmid5280 Thanks! Can you add the loading indicator so it's below the date field on this page? It's a little odd that the loading indicator moves the date field and displays at a different place than the content that is being loaded.

Then, I think the loading indicator where you've put it is a nice touch, but the issue actually referred to the track search when you load Show Builder for a particular show:

image

Can you add a loading indicator there as well?

kmid5280 commented 11 months ago

@seankwilliams Got it. Do you want to keep the new Loading component that was added as well?

It looks like the Loading component you are asking for needs to go on the ShowBuilderPage. Do we already have a conditional set up for that, so it can distinguish whether or not a search is currently being run? If not, do you have a suggestion on how best to implement it? We could define a state value called "searching," and in the searchLibrary function, set it to either true or false in the else condition, though I'm not sure if that would produce the desired behavior.

seankwilliams commented 11 months ago

@kmid5280 Yes, let's keep that Loading component on the Show Builder List Page, but let's move it so the loading indicator displays where the shows would display. Right now, it's pushing down the date box when it loads, and when it disappears, the content that loads is loading in a different place than the loading indicator was.

And yep, the new Loading component would need to go on the ShowBuilderPage. I am not sure if we have a conditional setup so you'd have to dig in and check. It's possible there is already a loading boolean in the Redux state that's associated with the searchLibrary action.

kmid5280 commented 11 months ago

@seankwilliams I was able to find the conditional in the library actions, and display the Loading component in the search tab while the content is loading. I set a position: relative attribute in the search tab so that the Loading component could be positioned absolutely inside of it.

One issue I'm running into is that the Loading component displays somewhat erratically. It will initially overlap the search box, and then subsequently overlap the previous search contents when a new search is run. This is probably because the Loading component is positioned absolutely. I'm not sure how to change that behavior in CSS with an absolutely positioned component like this - if I add a margin-top attribute for instance, then the positioning won't be consistent. Wonder if you have any thoughts?

seankwilliams commented 11 months ago

@kmid5280 usually with instances like this, while debugging, I'd remove all conditions that hide the Loading component. That way you can test different things using the inspector in the browser and try out different CSS to get the right thing. Then, once you have it right, you can conditionally show/hide the Loading component again.

kmid5280 commented 11 months ago

@seankwilliams I see that the Loading component has a displayMode attribute, so I changed that to static for this instance. See how that looks to you.

seankwilliams commented 11 months ago

@kmid5280 That is looking good! Can you make these two changes?

kmid5280 commented 11 months ago

@seankwilliams For the first change, do you want the Loading component displayed directly underneath the date field, or do you want it in the center of the page positioned below the date field?

seankwilliams commented 11 months ago

@kmid5280 let's center it

kmid5280 commented 11 months ago

@seankwilliams Okay I made some updates, see how that looks.

seankwilliams commented 11 months ago

This is looking great. Nice work @kmid5280 , I'll merge this into main when I do the next deployment!