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

New design for Masq #164

Merged
merged 41 commits into from
Apr 15, 2019
Merged

New design for Masq #164

merged 41 commits into from
Apr 15, 2019

Conversation

sujeebant
Copy link
Contributor

No description provided.

@sujeebant sujeebant force-pushed the favorite-list-masq-footer branch 2 times, most recently from 6030463 to ac9ef5d Compare April 3, 2019 12:36
@sujeebant sujeebant changed the title Masq Onboarding New design for Masq Apr 5, 2019
}

waitForClose() {
if (!this.opened) {

Choose a reason for hiding this comment

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

i don't understand the async of this method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The masq_favorite_modal is opened when a user that is not connected to masq tries to add a favorite to offer him to connect to masq. In the case he connects to masq through the modal, we have to add the favorite in his masq profile, otherwise we add it to the local storage as usual. That's why we need to wait for the modal to close before doing something in the poi panel, that's how the waitForClose promise is used.

There are alternative such as using events or just calling store.add in the modal component before closing so the poi_panel doesn't have to await the closing of the modal.

}

async previousStep () {
if (this.step === 1) {

Choose a reason for hiding this comment

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

i think there is a more elagant way to cycle over a limit list of integer

src/panel/favorites_panel.js Outdated Show resolved Hide resolved
@sujeebant sujeebant force-pushed the favorite-list-masq-footer branch 2 times, most recently from be94533 to 861cf16 Compare April 8, 2019 14:47
@sujeebant sujeebant requested review from Julien-laville and amatissart and removed request for Julien-laville April 8, 2019 14:47
Copy link
Contributor

@amatissart amatissart left a comment

Choose a reason for hiding this comment

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

A few questions about icons and images:

  • how is used masq.icon url ? and what is a better default than a github url ?
  • "masq-app".png seems to be unused
  • could we be optimize the gif files ? (not urgent though)

@@ -30,7 +30,7 @@ masq:
enabled: false
title: Qwant Maps
desc: Maps application developed by Qwant
icon: https://www.qwant.com/maps/statics/images/qwant-logo.svg
icon: https://raw.githubusercontent.com/QwantResearch/erdapfel/masq-new-image/public/images/masq/maps-app.png
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 including this image on this branch, and use it via a relative url for example ?

<br/>
<div class="modal__masq_favorite__details">
<i class="modal__masq_favorite__arrow"></i>
<a href="https://github.com/QwantResearch/masq-app" rel="noopener" target="_blank">{{= _('More Details') }}</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

should this url be configured ?

@amatissart amatissart dismissed Julien-laville’s stale review April 15, 2019 14:17

Approved after discussion

@amatissart amatissart merged commit 3b1edc1 into master Apr 15, 2019
@amatissart amatissart deleted the favorite-list-masq-footer branch May 3, 2019 09:15
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.

4 participants