esoko/mapguesser
esoko/mapguesser
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 18 Wiki Activity

feature/return-to-starting-point-button #26

Merged
balazs merged 9 commits from feature/return-to-starting-point-button into develop 2021-04-29 18:55:22 +02:00
Conversation 1 Commits 9 Files Changed 5 +11 -10
3 changed files with 11 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit ffc4aaf110 - Show all commits

7
public/static/css/game.css
View File

@ -157,13 +157,14 @@
#navigation .navigationItem {
margin-top: 10px;
opacity: 70%;
cursor: pointer;
bence commented 2021-04-29 00:19:10 +02:00
Outdated
Review
Copy Link
    cursor: pointer;

Then it will feel like a link.

```css cursor: pointer; ``` Then it will feel like a link.
}
#navigation .navigationItem:hover {
opacity: 100%;
}
bence commented 2021-04-29 00:32:38 +02:00
Outdated
Review
Copy Link

For semantical reasons I would use div instead of span because span is intended to be an inline element, however this a block element here.

    #navigation .navigationItem div {
For semantical reasons I would use `div` instead of `span` because `span` is intended to be an inline element, however this a block element here. ```css #navigation .navigationItem div { ```
balazs commented 2021-04-29 09:09:07 +02:00
Outdated
Review
Copy Link

I don't know why, but it breaks the visualization. I didn't see any difference in the CSS tree, when I tried it.

I don't know why, but it breaks the visualization. I didn't see any difference in the CSS tree, when I tried it.
balazs commented 2021-04-29 12:14:12 +02:00
Outdated
Review
Copy Link

I've realized I've left out the .navigationItem and that's why not the right div was selected. (I see now, that you wrote it correctly in your suggestion)

I've realized I've left out the .navigationItem and that's why not the right div was selected. (I see now, that you wrote it correctly in your suggestion)
#navigation span {
#navigation div {
position: absolute;
}
@ -188,9 +189,9 @@
}
#showGuessButtonContainer {
position: absolute;
bence commented 2021-04-29 00:13:05 +02:00
Outdated
Review
Copy Link

I would modify this as follows:

        left: 65px;

        right: 20px;

Then only left would change to leave space for the return to start button (and it would have a slightly bigger space).

I would modify this as follows: ```css left: 65px; ``` ```css right: 20px; ``` Then only `left` would change to leave space for the return to start button (and it would have a slightly bigger space). ![](https://i.ibb.co/7VRQz3Q/Bildschirmfoto-vom-2021-04-29-00-10-55.png)
balazs commented 2021-04-29 08:12:23 +02:00
Outdated
Review
Copy Link

I didn't want to break the symmetry, and I was thinking putting buttons to the right as well. But if you prefer the button to be larger over the symmetry, it's fine by me.

I didn't want to break the symmetry, and I was thinking putting buttons to the right as well. But if you prefer the button to be larger over the symmetry, it's fine by me.
bence commented 2021-04-29 18:27:07 +02:00
Outdated
Review
Copy Link

If further buttons are planned, it is OK for me.

If further buttons are planned, it is OK for me.
bence commented 2021-04-29 18:27:46 +02:00
Outdated
Review
Copy Link

However I see you already changed the position, then I wouldn't change back.

However I see you already changed the position, then I wouldn't change back.
left: 60px;
left: 65px;
bottom: 30px;
right: 60px;
right: 20px;
z-index: 2;
}
#guess {

6
public/static/js/game.js
View File

@ -334,6 +334,9 @@
document.getElementById('guess').style.visibility = null;
document.getElementById('guess').classList.remove('result');
// needs to be set visible after the show guess map hid it in mobile view
document.getElementById("navigation").style.visibility = 'visible';
Game.initialize();
},
@ -379,9 +382,6 @@
document.getElementById('currentRound').innerHTML = String(Game.rounds.length) + '/' + String(Game.NUMBER_OF_ROUNDS);
Game.loadPano(Game.panoId, Game.pov);
bence commented 2021-04-29 00:28:28 +02:00
Outdated
Review
Copy Link

This semantically belongs to reset (after line 334) and resetRound (after line 360). Yes it will be some ugly code duplication but easier to find there and I plan to refactor this JS in the future.

This semantically belongs to `reset` (after line 334) and `resetRound` (after line 360). Yes it will be some ugly code duplication but easier to find there and I plan to refactor this JS in the future.
// needs to be set visible after the show guess map hid it in mobile view
document.getElementById("navigation").style.visibility = 'visible';
},
handleErrorResponse: function (error) {

8
views/game.php
View File

@ -61,12 +61,12 @@
</div>
<div id="navigation">
<div id="returnToStart" class="navigationItem">
<span>
<div>
bence commented 2021-04-29 00:33:58 +02:00
Outdated
Review
Copy Link

https://gitea.e5tv.hu/esoko/mapguesser/pulls/26/files#issuecomment-197

<div>
[https://gitea.e5tv.hu/esoko/mapguesser/pulls/26/files#issuecomment-197](https://gitea.e5tv.hu/esoko/mapguesser/pulls/26/files#issuecomment-197) ```html <div> ```
<img src="<?= $_ENV['STATIC_ROOT'] ?>/img/circle_background.svg?rev=<?= REVISION ?>" alt="Circle Background" class="circleBackground" />
</span>
<span>
</div>
<div>
<img src="<?= $_ENV['STATIC_ROOT'] ?>/img/house-fill.svg?rev=<?= REVISION ?>" alt="Return to Start icon" class="navigationIcon" />
</span>
</div>
</div>
</div>
@endsection