Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

QMAPS-865 fix PJ phone numbers reveals #284

Merged
merged 4 commits into from
Jul 16, 2019
Merged

QMAPS-865 fix PJ phone numbers reveals #284

merged 4 commits into from
Jul 16, 2019

Conversation

xem
Copy link
Contributor

@xem xem commented Jun 26, 2019

No description provided.

@GuillaumeGomez
Copy link
Contributor

GuillaumeGomez commented Jun 26, 2019

Why did the package-lock changed?

No idea, I had to do npm install before creating this branch and it got pushed with my code, but I'm not able to revert it ... is it bad?

@@ -216,6 +216,11 @@ PoiPanel.prototype.emptyClickOnMap = function() {
}
};

PoiPanel.prototype.showPhone = function() {
document.querySelector(".poi_phone_container_hidden").style.display = "none"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not using the template system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean re-render all the template, just to show the number? It seemed overkill.
Also, in the category list, the phone number is revealed in the same way (CSS display none => block, and it seemed fine so I did the same)

Copy link
Contributor

Choose a reason for hiding this comment

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

No, just part of the template which is much more maintainable than having to handle the DOM ourselves.

src/views/poi_panel.dot Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Contributor

No idea, I had to do npm install before creating this branch and it got pushed with my code, but I'm not able to revert it ... is it bad?

Yep, quite bad considering none of those changes have anything to do with your PR.

@GuillaumeGomez
Copy link
Contributor

You still have the lock file changes to remove.

@amatissart amatissart merged commit 4f27cda into master Jul 16, 2019
@GuillaumeGomez GuillaumeGomez deleted the QMAPS-865 branch July 16, 2019 14:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants