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 +90 -2
balazs commented 2021-04-28 18:34:45 +02:00
Member
Copy Link

Added return to start feature in a form of a button. When clicked teleports back to the first panorama. Helps with finding the exact location and getting out stuck situations.

Added return to start feature in a form of a button. When clicked teleports back to the first panorama. Helps with finding the exact location and getting out stuck situations.
balazs added 3 commits 2021-04-28 18:34:45 +02:00
returns to start when clicked on home symbol
All checks were successful
default-pipeline default-pipeline #126
Details
5c8f3d6fb9
reformatted: added new line breaks to the game.css
All checks were successful
default-pipeline default-pipeline #127
Details
3d52c967b6
balazs requested review from bence 2021-04-28 18:34:55 +02:00
balazs closed this pull request 2021-04-28 20:56:39 +02:00
bence reviewed 2021-04-29 00:13:05 +02:00
public/static/css/game.css Outdated
@ -156,3 +189,3 @@
#showGuessButtonContainer {
position: absolute;
left: 20px;
left: 60px;
bence commented 2021-04-29 00:13:05 +02:00
Owner
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
Author
Member
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
Owner
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
Owner
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.
bence reviewed 2021-04-29 00:19:10 +02:00
public/static/css/game.css Outdated
@ -152,0 +157,4 @@
#navigation .navigationItem {
margin-top: 10px;
opacity: 70%;
}
bence commented 2021-04-29 00:19:10 +02:00
Owner
Copy Link
    cursor: pointer;

Then it will feel like a link.

```css cursor: pointer; ``` Then it will feel like a link.
bence reviewed 2021-04-29 00:28:28 +02:00
public/static/js/game.js Outdated
@ -381,1 +381,4 @@
Game.loadPano(Game.panoId, Game.pov);
// needs to be set visible after the show guess map hid it in mobile view
document.getElementById("navigation").style.visibility = 'visible';
bence commented 2021-04-29 00:28:28 +02:00
Owner
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.
bence reviewed 2021-04-29 00:32:38 +02:00
public/static/css/game.css Outdated
@ -152,0 +163,4 @@
opacity: 100%;
}
#navigation span {
bence commented 2021-04-29 00:32:38 +02:00
Owner
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
Author
Member
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
Author
Member
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)
bence reviewed 2021-04-29 00:33:58 +02:00
views/game.php Outdated
@ -61,1 +61,4 @@
</div>
<div id="navigation">
<div id="returnToStart" class="navigationItem">
<span>
bence commented 2021-04-29 00:33:58 +02:00
Owner
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> ```
bence commented 2021-04-29 00:55:24 +02:00
Owner
Copy Link

Okay, it turned out for me that this PR is already closed :D Maybe this could still be merged independently or the changes could be merged with #27.

Okay, it turned out for me that this PR is already closed :D Maybe this could still be merged independently or the changes could be merged with #27.
balazs reopened this pull request 2021-04-29 08:09:21 +02:00
balazs added 1 commit 2021-04-29 08:25:39 +02:00
fixed findings
All checks were successful
default-pipeline default-pipeline #137
Details
ffc4aaf110
balazs added 1 commit 2021-04-29 08:25:56 +02:00
Merge branch 'develop' into feature/return-to-starting-point-button
All checks were successful
default-pipeline default-pipeline #138
Details
f0b0de405c
balazs added 2 commits 2021-04-29 09:05:51 +02:00
balazs added 1 commit 2021-04-29 09:08:10 +02:00
balazs added 1 commit 2021-04-29 12:09:27 +02:00
replaced spans with divs again and fixed the css issue
All checks were successful
default-pipeline default-pipeline #142
Details
8f0cfd4489
bence approved these changes 2021-04-29 18:29:23 +02:00
balazs merged commit 759f654a0d into develop 2021-04-29 18:55:22 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
bence
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: esoko/mapguesser#26
No description provided.
Delete Branch "feature/return-to-starting-point-button"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?