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 16:55:22 +00:00
Conversation 7 Commits 9 Files Changed 5
+90 -2
balazs commented 2021-04-28 16:34:45 +00:00
Member
Copy link
Copy source

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 16:34:45 +00:00
added a new button to the view for the return to starting point feature b9927f79fc
returns to start when clicked on home symbol
default-pipeline default-pipeline #126
Details
5c8f3d6fb9
reformatted: added new line breaks to the game.css
default-pipeline default-pipeline #127
Details
3d52c967b6
balazs requested review from bence 2021-04-28 16:34:55 +00:00
balazs closed this pull request 2021-04-28 18:56:39 +00:00
public/static/css/game.css Outdated
@@ -156,3 +189,3 @@
#showGuessButtonContainer {
position: absolute;
left: 20px;
left: 60px;
bence commented 2021-04-28 22:13:05 +00:00
Owner
Copy link
Copy source

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 06:12:23 +00:00
Author
Member
Copy link
Copy source

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 16:27:07 +00:00
Owner
Copy link
Copy source

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 16:27:46 +00:00
Owner
Copy link
Copy source

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.
public/static/css/game.css Outdated
@@ -152,0 +157,4 @@
#navigation .navigationItem {
margin-top: 10px;
opacity: 70%;
}
bence commented 2021-04-28 22:19:10 +00:00
Owner
Copy link
Copy source
    cursor: pointer;

Then it will feel like a link.

```css cursor: pointer; ``` Then it will feel like a link.
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-28 22:28:28 +00:00
Owner
Copy link
Copy source

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.
public/static/css/game.css Outdated
@@ -152,0 +163,4 @@
opacity: 100%;
}
#navigation span {
bence commented 2021-04-28 22:32:38 +00:00
Owner
Copy link
Copy source

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 07:09:07 +00:00
Author
Member
Copy link
Copy source

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 10:14:12 +00:00
Author
Member
Copy link
Copy source

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)
views/game.php Outdated
@@ -61,1 +61,4 @@
</div>
<div id="navigation">
<div id="returnToStart" class="navigationItem">
<span>
bence commented 2021-04-28 22:33:58 +00:00
Owner
Copy link
Copy source

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-28 22:55:24 +00:00
Owner
Copy link
Copy source

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 06:09:21 +00:00
balazs added 1 commit 2021-04-29 06:25:39 +00:00
fixed findings
default-pipeline default-pipeline #137
Details
ffc4aaf110
balazs added 1 commit 2021-04-29 06:25:56 +00:00
Merge branch 'develop' into feature/return-to-starting-point-button
default-pipeline default-pipeline #138
Details
f0b0de405c
balazs added 2 commits 2021-04-29 07:05:51 +00:00
reverted replacing spans with divs 3f4fa83d87
Merge branch 'feature/return-to-starting-point-button' of gitea.e5tv.hu:esoko/mapguesser into feature/return-to-starting-point-button
default-pipeline default-pipeline #139
Details
462f6cde80
balazs added 1 commit 2021-04-29 07:08:10 +00:00
added the visibility reset to resetRound and reverted the span div replacement in the view
default-pipeline default-pipeline #140
Details
3122696364
balazs added 1 commit 2021-04-29 10:09:27 +00:00
replaced spans with divs again and fixed the css issue
default-pipeline default-pipeline #142
Details
8f0cfd4489
bence approved these changes 2021-04-29 16:29:23 +00:00
balazs merged commit 759f654a0d into develop 2021-04-29 16:55:22 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
bence
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
balazs (Balázs Vigh) bence (Bence Pőcze) system-user-ci
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: esoko/mapguesser#26