CSI-280 / MusicGames

0 stars 0 forks source link

Create a Navbar #13

Closed JakeCapra closed 4 years ago

JakeCapra commented 4 years ago

I mentioned this in our first meeting, but since our project is music-based, it would be cool if we could have a music player in the navbar, so music was playing on every page you visited, and it would be continuous. Last.fm does something similar: image https://wireframe.cc/AxfMXd The wireframe I made uses some of what Lenora made in issue #2

chrisbendel commented 4 years ago

In terms of implementation (i've built something exactly like this before)

If you use react router, make sure you keep this music component at the top level of your component hierarchy, and every component that will change will be inside the router on the same level: example: https://github.com/chrisbendel/hose/blob/master/src/App.js#L56

jbuzzell commented 4 years ago

@JakeCapra there's a music player already laid out for the bottom corner of any given page in #2 and #8. i think it would make more sense for this to be at the bottom rather than to be a component of the header--try and make sure each content area only serves one purpose (i.e. the header provides navigation, the game box is where gameplay happens, etc)

JakeCapra commented 4 years ago

@jbuzzell It is much easier if we put it in the navbar. If we put it in the bottom, continuous playing becomes much more difficult. Basically, if we put it in the navbar, the player will not change when navigating pages. If we put it in the bottom, we have to overlay it over the page that is being rendered. I think we could try putting it in the bottom on a later date when we don't have so much work to do.

jbuzzell commented 4 years ago

the player will still reset itself on every page by nature of the fact that when you go to a new page, you are loading a new document. there are ways to maintain persistence across pages in JS (mainly faking that you're moving to a new page without actually doing so & generating a history entry), so it's possible, but putting the player in the header or anywhere else will not affect its ability to maintain state continuously. if there is some sort of persistence smoke and mirrors already in effect, then that's my mistake. however, if that is the case then it should be even easier to put the player elsewhere.

JakeCapra commented 4 years ago

We're using react-router-dom. Which means the navbar will not be re-rendered when we change pages. This is why react is awesome.

jbuzzell commented 4 years ago

gotcha, missed that. i'd still like to consider it being elsewhere (or perhaps retooling the navbar to look a little sleeker with the player included) in the future, but for now that's fine. just want to make sure everything is readable in the end

jbuzzell commented 4 years ago

adding some CSS, going to sneak in some rules from #20 into this PR

jbuzzell commented 4 years ago

pushed css, created PR #32

JakeCapra commented 4 years ago

I’m not done yet :/

jbuzzell commented 4 years ago

oops! sorry! any commits you make to this branch before it gets pushed will get added to the PR

On Mon, Mar 30, 2020, 6:56 PM Jake Capra notifications@github.com wrote:

I’m not done yet :/

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CSI-280/MusicGames/issues/13#issuecomment-606294217, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG6ILDHYCUQVFRN2VLBDQ5LRKEPQFANCNFSM4LTRY5FA .

JakeCapra commented 4 years ago

@jbuzzell did you look at the css changes you made? I feel like the navbar is too big. Also, the title isn't centered (it wasn't before either). I also think it is wise to use viewport font sizes as I mentioned before, because it adapts to the size of the screen. image

jbuzzell commented 4 years ago

i've used viewport font sizes before and generally thought they caused more trouble than they were worth--sometimes aligning to screen size can have unintended consequences. i would have to play around with them a little bit in order to convert them to viewport, but i would be willing if folks thought it was necessary.

my bad on the title, i thought it was meant to be left aligned.

On Mon, Mar 30, 2020, 8:37 PM Jake Capra notifications@github.com wrote:

@jbuzzell https://github.com/jbuzzell did you look at the css changes you made? I feel like the navbar is too big. Also, the title isn't centered (it wasn't before either). I also think it is wise to use viewport https://css-tricks.com/viewport-sized-typography/font sizes as I mentioned before, because it adapts to the size of the screen. [image: image] https://user-images.githubusercontent.com/49655704/77974731-028b5c00-72c6-11ea-92ff-1f0af25900f0.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/CSI-280/MusicGames/issues/13#issuecomment-606326502, or unsubscribe https://github.com/notifications/unsubscribe-auth/AG6ILDGT3EM4WJXIMJ2R4MTRKE3MXANCNFSM4LTRY5FA .