Added Navbar to the Website. #336

Vedant-Manjrekar · 2022-10-03T14:23:32Z

🛠️ Fixes #106

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

✔️ Check List (Check all the applicable boxes)

My code follows the code style of this project.
This PR does not contain plagiarized content.
The title of my pull request is a short description of the requested changes.

accesslint

There are accessibility issues in these changes.

accesslint · 2022-10-03T14:23:38Z

index.html

+               <a href="/lyrics" id="lyrics-btn" class="btn">Lyrics/Individual Song Page</a>   
+               <a href="/you-may-like" class="btn" id="yml" target="_blank">You May Also Like</a>
+               <h1 class="heading g-col-4">A very Mogul Christmas</h1>
+               <button class="navbar-toggler toggler" type="button" data-bs-toggle="offcanvas" data-bs-target="#offcanvasNavbar" aria-controls="offcanvasNavbar">


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-03T14:23:38Z

index.html

+               <div id="header">
+                  <div id="switch">
+                     <label class="switch">
+                     <input type="checkbox">


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

netlify · 2022-10-03T14:23:46Z

✅ Deploy Preview for mogulchristmas ready!

Name	Link
🔨 Latest commit	`6974e58`
🔍 Latest deploy log	https://app.netlify.com/sites/mogulchristmas/deploys/634555af2534140009dddb19
😎 Deploy Preview	https://deploy-preview-336--mogulchristmas.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

KendallDoesCoding

Desktop View

Toggle of Darkmode button as been removed
The text "A Very Mogul Christmas" is not perfectly aligned in the center.. Due to this, when you scroll down you can see the text, but I don't want it to be this way, I like how the original looked.:
I don't really like the color of the buttons on the top left, can you check this site out (https://cssgradient.io/gradient-backgrounds/) and make the buttons a color that goes well with Pink? (Don't make both of them the same color, but around the same will do, ie. one be pink and purple, one be pink and violet, it just should look different!)
For the buttons in the top left corner, can you make the background a little transparent?
For the Rate Song and Other Similar Projects button (keep the color as is, Violet and Orange (no need to make it Blue))
For the top left button, if possible can you make the text -"If you liked this album, you may also like", if not then You May Like will do.

Mobile View

Looks amazing, just a few edits:

The Lyrics/Individual Song Page button text background is green color, instead of white:
For the second button, instead of You May Like can you make it If you liked this album, You May Also Like?
The toggle for darkmode on mobile view, can be in the left hand corner.
The menu should have the background color as is (located in the /images directory), and the toggle button to change between backgrounds.

That's all I can think of for now, please do these edits and will have to look again.

I know, it's a lot of edits, so please take your time.

Sorry for the inconvenience,
Kendall Does Coding

KendallDoesCoding · 2022-10-03T15:17:45Z

@TechStudent10 Feel free to share your thoughts on this as well.

accesslint

There are accessibility issues in these changes.

accesslint · 2022-10-04T07:31:41Z

index.html

+            >If you liked this album, You May Also Like</a
+          >
+          <h1 class="heading g-col-4">A very Mogul Christmas</h1>
+          <button


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-04T07:31:41Z

index.html

+            <div id="header">
+              <div id="switch">
+                <label class="switch">
+                  <input type="checkbox" />


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-04T07:31:41Z

index.html

+    </div>
+    <!-- Loop button -->
+    <div class="random-text-container">
+      <button class="loop-btn-style loop-false" id="loop-btn">


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

ghost · 2022-10-04T07:33:32Z

index.js

@@ -46,6 +47,19 @@ const updateMode = () => {
  isLight
    ? (slider.style.backgroundImage = "url('./images/light.png')")
    : (slider.style.backgroundImage = "url('./images/dark.png')");
+
+  isLight


[ESLint] Expected an assignment or function call and instead saw an expression. (view)

Rule Severity Recommended

no-unused-expressions warn No

References:

https://eslint.org/docs/rules/no-unused-expressions

You can close this issue if no need to fix it. Learn more.

ghost · 2022-10-04T07:33:32Z

index.js

+    ? (canvasBody.style.backgroundColor = "#fdd7d1")
+    : (canvasBody.style.backgroundColor = "#dadada");
+
+  isLight


[ESLint] Expected an assignment or function call and instead saw an expression. (view)

Rule Severity Recommended

no-unused-expressions warn No

References:

https://eslint.org/docs/rules/no-unused-expressions

You can close this issue if no need to fix it. Learn more.

ghost · 2022-10-04T07:33:33Z

index.js

+    ? (canvasHead.style.backgroundColor = "#fdd7d1")
+    : (canvasHead.style.backgroundColor = "#dadada");
+
+  isLight


[ESLint] Expected an assignment or function call and instead saw an expression. (view)

Rule Severity Recommended

no-unused-expressions warn No

References:

https://eslint.org/docs/rules/no-unused-expressions

You can close this issue if no need to fix it. Learn more.

ghost · 2022-10-04T07:33:33Z

index.js

-    }
-    else{
+function loopWatcher(start, end, apiURL) {
+  const waitTime = (parseInt(end) - parseInt(start) + 5) * 1000; //seconds


[ESLint] Missing radix parameter. (view)

Rule Severity Recommended

radix warn No

References:

https://eslint.org/docs/rules/radix

You can close this issue if no need to fix it. Learn more.

Vedant-Manjrekar · 2022-10-04T07:40:03Z

@KendallDoesCoding I made the changes you asked and submitted PR for the same, have a look.

KendallDoesCoding · 2022-10-04T08:35:46Z

@KendallDoesCoding I made the changes you asked and submitted PR for the same, have a look.

You haven't done everything yet, but looks much better.

KendallDoesCoding

Desktop View

The text "A Very Mogul Christmas" on the first line is still not centered
Darkmode is removed.
I want the "Rate Song" and the "Other Similar Projects" not as buttons but just as text with color, like the current website is.

Mobile View

In the menu, the text You May Like is bold, and the toggle changes it to normal, please make the text normal first.
Please include darkmode in the left hand side.

Other then that, it seems to be good! The darkmode is very important.

…ll, Heading is centered and tweeks with font colors.

accesslint

There are accessibility issues in these changes.

accesslint · 2022-10-04T11:54:23Z

index.html

+          <div id="header">
+            <div id="switch">
+              <label class="switch">
+                <input type="checkbox" />


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-04T11:54:23Z

index.html

-               <button class="stop-btn-style" id="stop-btn" onClick="stopVideo()">Stop</button>
+          </div>
+
+          <button


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-04T11:54:23Z

index.html

+            <div id="header">
+              <div id="switch1">
+                <label class="switch">
+                  <input type="checkbox" />


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-04T11:54:23Z

index.html

+    </div>
+    <!-- Loop button -->
+    <div class="random-text-container">
+      <button class="loop-btn-style loop-false" id="loop-btn">


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

ghost · 2022-10-04T11:56:45Z

index.js

+    ? (toggle1.style.backgroundImage = "url('./images/light.png')")
+    : (toggle1.style.backgroundImage = "url('./images/dark.png')");
+
+  isLight


[ESLint] Expected an assignment or function call and instead saw an expression. (view)

Rule Severity Recommended

no-unused-expressions warn No

References:

https://eslint.org/docs/rules/no-unused-expressions

You can close this issue if no need to fix it. Learn more.

ghost · 2022-10-04T11:56:45Z

index.js

+    ? (canvasBody.style.backgroundColor = "#fdd7d1")
+    : (canvasBody.style.backgroundColor = "#dadada");
+
+  isLight


[ESLint] Expected an assignment or function call and instead saw an expression. (view)

Rule Severity Recommended

no-unused-expressions warn No

References:

https://eslint.org/docs/rules/no-unused-expressions

You can close this issue if no need to fix it. Learn more.

ghost · 2022-10-04T11:56:45Z

index.js

+    ? (canvasHead.style.backgroundColor = "#fdd7d1")
+    : (canvasHead.style.backgroundColor = "#dadada");
+
+  isLight


[ESLint] Expected an assignment or function call and instead saw an expression. (view)

Rule Severity Recommended

no-unused-expressions warn No

References:

https://eslint.org/docs/rules/no-unused-expressions

You can close this issue if no need to fix it. Learn more.

ghost · 2022-10-04T11:56:45Z

index.js

-    }
-    else{
+function loopWatcher(start, end, apiURL) {
+  const waitTime = (parseInt(end) - parseInt(start) + 5) * 1000; //seconds


[ESLint] Missing radix parameter. (view)

Rule Severity Recommended

radix warn No

References:

https://eslint.org/docs/rules/radix

You can close this issue if no need to fix it. Learn more.

ghost · 2022-10-04T11:56:46Z

index.js

@@ -46,6 +50,23 @@ const updateMode = () => {
  isLight
    ? (slider.style.backgroundImage = "url('./images/light.png')")
    : (slider.style.backgroundImage = "url('./images/dark.png')");
+
+  isLight


[ESLint] Expected an assignment or function call and instead saw an expression. (view)

Rule Severity Recommended

no-unused-expressions warn No

References:

https://eslint.org/docs/rules/no-unused-expressions

You can close this issue if no need to fix it. Learn more.

KendallDoesCoding

Perfect!

Please add the darkmode toggle button on Mobile View in the left and then I may change the color of the buttons, and we're good to go to merge.

Vedant-Manjrekar · 2022-10-04T12:04:15Z

Its in left already, isn't it ?

KendallDoesCoding · 2022-10-04T12:23:42Z

Nono, not the menu, the main screen/page. I require darkmode there on the left.

Vedant-Manjrekar · 2022-10-04T12:31:31Z

Send screenshot of the change you want.

TechStudent10 · 2022-10-04T23:48:59Z

I like the concept

Vedant-Manjrekar · 2022-10-08T08:40:32Z

Send screenshot of the change you want.

@KendallDoesCoding why there is no response to it? Don't you want the changes?

KendallDoesCoding · 2022-10-08T08:46:29Z

Send screenshot of the change you want.

@KendallDoesCoding why there is no response to it? Don't you want the changes?

Sorry, I got busy with exams.

KendallDoesCoding · 2022-10-08T08:46:55Z

Send screenshot of the change you want.

I want the button on the home page, if possible.

Currently, users have to go to the menu, and toggle it on a off.

KendallDoesCoding · 2022-10-08T08:47:04Z

If not possible, then will go to merge.

KendallDoesCoding · 2022-10-08T08:47:19Z

I like the concept

Yup, same!!

Vedant-Manjrekar · 2022-10-08T12:11:02Z

If not possible, then will go to merge.

It's possible, but the thing is since website's name "a very mogul Christmas" is already a big and adding another button next to it might ruin the aesthetics of the navbar due to overload of things as there is not much space in mobile view, but if aesthetics aren't an issue I can add it as you want.

KendallDoesCoding · 2022-10-08T12:13:46Z

If not possible, then will go to merge.

It's possible, but the thing is since website's name "a very mogul Christmas" is already a big and adding another button next to it might ruin the aesthetics of the navbar due to overload of things as there is not much space in mobile view, but if aesthetics aren't an issue I can add it as you want.

It's not really a issue but should look decent, can you do it and share a screenshot, if that's not a problem?

KendallDoesCoding · 2022-10-09T13:25:03Z

If not possible, then will go to merge.

It's possible, but the thing is since website's name "a very mogul Christmas" is already a big and adding another button next to it might ruin the aesthetics of the navbar due to overload of things as there is not much space in mobile view, but if aesthetics aren't an issue I can add it as you want.

It's not really a issue but should look decent, can you do it and share a screenshot, if that's not a problem?

Waiting for a update on this @Vedant-Manjrekar

KendallDoesCoding · 2022-10-11T11:11:53Z

@Vedant-Manjrekar Please do the changes as soon as possible. This is a very important feature that I require to add.

accesslint

There are accessibility issues in these changes.

accesslint · 2022-10-11T11:25:44Z

index.html

+          <div id="header1">
+            <div id="switch1">
+              <label class="switch">
+                <input type="checkbox" />


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-11T11:25:44Z

index.html

+          <div id="header">
+            <div id="switch">
+              <label class="switch">
+                <input type="checkbox" />


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-11T11:25:44Z

index.html

-      </div>
+          </div>
+
+          <button


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

Vedant-Manjrekar · 2022-10-11T11:30:24Z

If not possible, then will go to merge.

It's possible, but the thing is since website's name "a very mogul Christmas" is already a big and adding another button next to it might ruin the aesthetics of the navbar due to overload of things as there is not much space in mobile view, but if aesthetics aren't an issue I can add it as you want.

It's not really a issue but should look decent, can you do it and share a screenshot, if that's not a problem?

Waiting for a update on this @Vedant-Manjrekar

Done

accesslint

There are accessibility issues in these changes.

accesslint · 2022-10-11T11:38:27Z

index.html

+          <div id="header1">
+            <div id="switch1">
+              <label class="switch">
+                <input type="checkbox" />


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-11T11:38:28Z

index.html

+          <div id="header">
+            <div id="switch">
+              <label class="switch">
+                <input type="checkbox" />


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

accesslint · 2022-10-11T11:38:28Z

index.html

-      </div>
+          </div>
+
+          <button


Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

KendallDoesCoding · 2022-10-11T15:20:02Z

Perfect! Thanks for your contribution.

KendallDoesCoding · 2022-10-11T15:20:33Z

@all-contributors please add @Vedant-Manjrekar to the doc

allcontributors · 2022-10-11T15:20:43Z

@KendallDoesCoding

I've put up a pull request to add @Vedant-Manjrekar! 🎉

Vedant-Manjrekar · 2022-10-12T04:39:46Z

Perfect! Thanks for your contribution.

You are most welcome.

Added Navbar to the Website.

cd65627

Vedant-Manjrekar requested a review from KendallDoesCoding as a code owner October 3, 2022 14:23

pull-request-size bot added the size/L label Oct 3, 2022

accesslint bot reviewed Oct 3, 2022

View reviewed changes

KendallDoesCoding reviewed Oct 3, 2022

View reviewed changes

KendallDoesCoding requested a review from TechStudent10 October 3, 2022 15:17

Made changes in UI as asked.

27c01af

pull-request-size bot added size/XL and removed size/L labels Oct 4, 2022

accesslint bot reviewed Oct 4, 2022

View reviewed changes

ghost reviewed Oct 4, 2022

View reviewed changes

KendallDoesCoding self-requested a review October 4, 2022 08:35

KendallDoesCoding requested changes Oct 4, 2022

View reviewed changes

Made changes in UI, now toggler is available for desktop devices aswe…

aaa56ad

…ll, Heading is centered and tweeks with font colors.

accesslint bot reviewed Oct 4, 2022

View reviewed changes

Vedant-Manjrekar requested review from KendallDoesCoding and removed request for TechStudent10 October 4, 2022 11:56

ghost reviewed Oct 4, 2022

View reviewed changes

KendallDoesCoding requested changes Oct 4, 2022

View reviewed changes

Added theme toggle button outside the menu drawer.

50229a7

accesslint bot reviewed Oct 11, 2022

View reviewed changes

Ressolved merge conflicts.

6974e58

accesslint bot reviewed Oct 11, 2022

View reviewed changes

Vedant-Manjrekar requested a review from KendallDoesCoding October 11, 2022 11:41

KendallDoesCoding approved these changes Oct 11, 2022

View reviewed changes

KendallDoesCoding merged commit 820b6ca into KendallDoesCoding:main Oct 11, 2022

allcontributors bot mentioned this pull request Oct 11, 2022

docs: add Vedant-Manjrekar as a contributor for doc #337

Merged

Vedant-Manjrekar deleted the vedant_navbar branch October 12, 2022 04:39

-                    </div>
+                        </div>
+                        <button

Added Navbar to the Website. #336

Added Navbar to the Website. #336

Conversation

Vedant-Manjrekar commented Oct 3, 2022 • edited by KendallDoesCoding Loading

🛠️ Fixes #106

✔️ Check List (Check all the applicable boxes)

accesslint bot left a comment

Choose a reason for hiding this comment

accesslint bot Oct 3, 2022

Choose a reason for hiding this comment

accesslint bot Oct 3, 2022

Choose a reason for hiding this comment

netlify bot commented Oct 3, 2022 • edited Loading

✅ Deploy Preview for mogulchristmas ready!

KendallDoesCoding left a comment

Choose a reason for hiding this comment

Desktop View

Mobile View

KendallDoesCoding commented Oct 3, 2022

accesslint bot left a comment

Choose a reason for hiding this comment

accesslint bot Oct 4, 2022

Choose a reason for hiding this comment

accesslint bot Oct 4, 2022

Choose a reason for hiding this comment

accesslint bot Oct 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vedant-Manjrekar commented Oct 4, 2022

KendallDoesCoding commented Oct 4, 2022

KendallDoesCoding left a comment

Choose a reason for hiding this comment

Desktop View

Mobile View

accesslint bot left a comment

Choose a reason for hiding this comment

accesslint bot Oct 4, 2022

Choose a reason for hiding this comment

accesslint bot Oct 4, 2022

Choose a reason for hiding this comment

accesslint bot Oct 4, 2022

Choose a reason for hiding this comment

accesslint bot Oct 4, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KendallDoesCoding left a comment

Choose a reason for hiding this comment

Vedant-Manjrekar commented Oct 4, 2022

KendallDoesCoding commented Oct 4, 2022

Vedant-Manjrekar commented Oct 4, 2022 • edited Loading

TechStudent10 commented Oct 4, 2022

Vedant-Manjrekar commented Oct 8, 2022

KendallDoesCoding commented Oct 8, 2022

KendallDoesCoding commented Oct 8, 2022

KendallDoesCoding commented Oct 8, 2022

KendallDoesCoding commented Oct 8, 2022

Vedant-Manjrekar commented Oct 8, 2022

KendallDoesCoding commented Oct 8, 2022

KendallDoesCoding commented Oct 9, 2022

KendallDoesCoding commented Oct 11, 2022

accesslint bot left a comment

Choose a reason for hiding this comment

accesslint bot Oct 11, 2022

Choose a reason for hiding this comment

accesslint bot Oct 11, 2022

Choose a reason for hiding this comment

accesslint bot Oct 11, 2022

Choose a reason for hiding this comment

Vedant-Manjrekar commented Oct 11, 2022

accesslint bot left a comment

Choose a reason for hiding this comment

accesslint bot Oct 11, 2022

Choose a reason for hiding this comment

Vedant-Manjrekar commented Oct 3, 2022 •

edited by KendallDoesCoding

Loading

netlify bot commented Oct 3, 2022 •

edited

Loading

Vedant-Manjrekar commented Oct 4, 2022 •

edited

Loading