Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add underscore js and bootstrap css #2

Merged
merged 2 commits into from
Jul 8, 2018

Conversation

danylenkoivan
Copy link
Contributor

No description provided.

Copy link
Owner

@sharifmarat sharifmarat left a comment

Choose a reason for hiding this comment

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

I think it's a good direction, but could you fix at least event's data?

action: 'remove_guest',
id: guest_id
});
if (confirm('Are you sure you want to remove ' + name + '. This is permanent!')) {
Copy link
Owner

Choose a reason for hiding this comment

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

There was a discussion in a group regarding that. Some people suggested to type a word to actually confirm changes. Not sure if just a confirmation would be enough. Maybe use 'prompt' for removing at least?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess typing isn't comfortable while using mobile device?

Copy link
Owner

Choose a reason for hiding this comment

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

They wanted to be on a very safe side to not delete by an accident

if (i == 0) {
$('#event_date').text(result.message[i].date);
$('#event_location').text(result.message[i].location);
$('#payment_link').text('TBD');
Copy link
Owner

Choose a reason for hiding this comment

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

#event_date, #event_location, #payment_link are not updated after the change anymore

@@ -4,50 +4,80 @@
<meta name="viewport" content="width=device-width">
<title>Let's volleyball</title>
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Lora:400,700,400italic,700italic|Anonymous+Pro:400,700,400italic,700italic|Merriweather:400,700,300">
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">
<link rel="stylesheet" href="css/main.css?v=1.0.2">
Copy link
Owner

Choose a reason for hiding this comment

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

I guess many things from main.css could be removed?

@@ -4,50 +4,80 @@
<meta name="viewport" content="width=device-width">
<title>Let's volleyball</title>
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Lora:400,700,400italic,700italic|Anonymous+Pro:400,700,400italic,700italic|Merriweather:400,700,300">
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">
Copy link
Owner

Choose a reason for hiding this comment

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

Is it better to use bootstrapcdn or get it locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't matter. Having source outside makes less impact on the server, having it locally is more reliable, however in case with boostrap CDN it shouldn't be an issue.

@@ -4,50 +4,80 @@
<meta name="viewport" content="width=device-width">
<title>Let's volleyball</title>
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Lora:400,700,400italic,700italic|Anonymous+Pro:400,700,400italic,700italic|Merriweather:400,700,300">
<link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0/css/bootstrap.min.css" integrity="sha384-Gn5384xqQ1aoWXA+058RXPxPg6fy4IWvTNh0E263XmFcJlSAwiGgFAW/dAiS6JXm" crossorigin="anonymous">
Copy link
Owner

Choose a reason for hiding this comment

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

A few comments based on this image
fields_collision_edited

  1. Event data is not updated (important)
  2. Paid checkbox is on position input (less critical)
  3. Position iput is under a button (less critical)

@sharifmarat sharifmarat merged commit 08525ca into sharifmarat:master Jul 8, 2018
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.

2 participants