Skip to content

Conversation

@JoseBambora
Copy link
Contributor

Made the navbar mobile friendly, fixed a typo error and changed the name "performancelive" to "karaoke_performance_id" in order to be more intuitive of what it represent.

Copy link
Member

@ruioliveira02 ruioliveira02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I click on the go to button on the home page nothing happens and the button disappears. This happens only in the unauthenticated pages.

image
I think the dropdown should start right after the button ends vertically, as to not have any overlap which makes it a bit awkward to use

Copy link
Member

@ruioliveira02 ruioliveira02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
On mobile it is off center (there is some extra padding coming from the <ul> tags.

Comment on lines 92 to 94
<ul>
<a href={subentry.url}><%= subentry.title %></a>
</ul>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should not be using <ul> here. If anything, it should be a <li> tag and you put the <ul> on top of the for loop

<ul>
  <%= for .... do %>
    <li>
      ...
    </li>
  <% end %>
</ul>

Copy link
Member

@ruioliveira02 ruioliveira02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to format / lint your code so the checks pass

Comment on lines 17 to 60
<script>
var isClicked = false
function pixeis(){
var result = false
if(window.innerWidth < 700)
result = true
return result
}
function scene1()
{
document.getElementById("menu2").className = "text-center"
document.getElementById("content").className = "hidden"
document.getElementById("menu").className = "hidden"
}
function scene2()
{
document.getElementById("menu").className = "text-center"
document.getElementById("menu2").className = "hidden"
document.getElementById("content").className = "container"
}
function clickedbutton() {
isClicked = !isClicked
if(isClicked){
if(pixeis())
scene1();
else
scene2();
}
else{
document.getElementById("menu").className = "hidden"
document.getElementById("menu2").className = "hidden"
document.getElementById("content").className = "container"
}
}
function resized()
{
if(isClicked)
if(pixeis())
scene1()
else
scene2()
}
window.onresize = resized;
</script>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a fan of this. Maybe we can use Alpine.js and Tailwind's breakpoints to clean this up (@nelsonmestevao)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be really cool to use Alpine and tailwind breakpoints. Should I add alpine to this project?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think it would be a good addition

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants