Skip to content

Commit

Permalink
FXVPN-141 Nits for the serverlist (#60)
Browse files Browse the repository at this point in the history
  • Loading branch information
strseb authored Sep 24, 2024
1 parent 2bd723a commit 200f896
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 37 deletions.
10 changes: 9 additions & 1 deletion src/_locales/en/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@
"description": "Alt text for the button that opens the Extensions Settings page"
},
"searchServer": {
"message": "Search Server",
"message": "Search Location",
"description": "Placeholder for the Input field Where users can search the Serverlist"
},
"titleSubscribeNow": {
Expand Down Expand Up @@ -157,5 +157,13 @@
"btnDownloadNow": {
"message": "Download now",
"description": "This is a call to action button to download the VPN."
},
"defaultLocationHeader": {
"message": "Use default Mozilla VPN Location",
"description": "Header for useDefaultLocationExplainer"
},
"useDefaultLocationExplainer": {
"message": "This is always the same location as selected in your Mozilla VPN desktop app.",
"description": "Explanation of what 'Use default Mozilla VPN Location' means."
}
}
69 changes: 63 additions & 6 deletions src/components/serverlist.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
styleMap,
createRef,
ref,
when,
} from "../vendor/lit-all.min.js";
import { ghostButtonStyles, resetSizing } from "./styles.js";

Expand Down Expand Up @@ -126,6 +127,28 @@ export class ServerList extends LitElement {
@change=${() => this.requestUpdate()}
@input=${() => this.requestUpdate()}
/>
<label
class="default-location-item"
@click=${() =>
when(this.selectedCity != null, () => {
this.dispatchEvent(this.#changeCityEvent(null));
})}
>
<input
class="default-location-btn"
type="radio"
.checked=${this.selectedCity == null}
/>
<span class="defaultCitySection">
<span class="default-location-headline"
>${tr("defaultLocationHeader")}</span
>
<span class="default-location-subline">
${tr("useDefaultLocationExplainer")}
</span>
</span>
</label>
<hr />
${countrylistHolder(filteredList, countryListProvider)}
`;
}
Expand Down Expand Up @@ -159,21 +182,28 @@ export class ServerList extends LitElement {
pointer-events: none;
}
.default-location-headline,
.server-country-name {
font-family: var(--font-family);
font-weight: 600;
font-size: 16px;
line-height: 24px;
}
.server-country-name {
padding-block: 0;
padding-inline-end: 0;
padding-inline-start: 20px;
font-family: var(--font-family);
font-weight: bold;
pointer-events: none;
color: var(--text-color-primary);
}
.default-location-item,
.server-city-list-item,
.server-city-list-visibility-btn {
display: flex;
flex-direction: row;
block-size: 40px;
border-radius: 4px;
margin-block-start: 4px;
margin-block-end: 4px;
Expand All @@ -182,6 +212,20 @@ export class ServerList extends LitElement {
inline-size: calc(100% - 16px);
position: relative;
}
.server-city-list-item,
.server-city-list-visibility-btn {
block-size: 40px;
}
.default-location-item {
padding: 0px 16px;
border-bottom: 1px solid var(--border-color);
border-radius: 0px;
padding-bottom: 16px;
}
.default-location-item input {
margin-right: 16px;
}
/* We need to temporarily use !important for this button to make sure the right color applies */
.server-city-list-visibility-btn {
Expand All @@ -200,6 +244,11 @@ export class ServerList extends LitElement {
inline-size: 24px;
height: 24px;
}
@media (prefers-color-scheme: dark) {
.toggle {
filter: invert(1);
}
}
.opened .toggle {
transform: rotate(0deg);
Expand All @@ -217,6 +266,14 @@ export class ServerList extends LitElement {
margin-left: 48px;
}
.default-location-subline,
.server-city-name {
font-weight: 400;
font-size: 14px;
line-height: 21px;
opacity: 0.875;
}
.opened .server-city-list {
opacity: 1;
visibility: visible;
Expand Down Expand Up @@ -251,11 +308,11 @@ export class ServerList extends LitElement {
input.search {
margin-block: 16px;
padding: 10px 20px 10px 30px;
padding: 10px 20px 10px 32px;
color: var(--text-color-invert);
width: calc(max(50%, 312px));
background-image: url("../../assets/img/search-icon.svg");
background-position: 2.5px 6px;
background-position: 5px 6px;
background-repeat: no-repeat;
border: 2px solid var(--border-color);
border-radius: var(--button-border-radius);
Expand Down Expand Up @@ -292,7 +349,7 @@ export const cityItem = (city, selectedCity, selectCity) => {
<input
class="server-radio-btn"
type="radio"
.checked=${city.name === selectedCity.name}
.checked=${city.name === selectedCity?.name}
data-country-code="${city.code}"
data-city-name="${city.name}"
/>
Expand Down
8 changes: 6 additions & 2 deletions src/shared/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ export const Utils = {
return false;
}
},
nameFor: (countryCode = "de", cityCode = "ber", serverList = []) => {
nameFor: (countryCode = "", cityCode = "", serverList = []) => {
return Utils.getCity(countryCode, cityCode, serverList)?.name;
},

getCity: (countryCode = "", cityCode = "", serverList = []) => {
if (!serverList) {
return "";
}
Expand All @@ -66,6 +70,6 @@ export const Utils = {
}
return serverList
.find((sc) => sc.code === countryCode)
?.cities.find((c) => c.code === cityCode)?.name;
?.cities.find((c) => c.code === cityCode);
},
};
71 changes: 45 additions & 26 deletions src/ui/browserAction/popupPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import "./../../components/titlebar.js";
import "./../../components/iconbutton.js";
import { SiteContext } from "../../background/proxyHandler/siteContext.js";
import {
ServerCity,
ServerCountry,
VPNState,
} from "../../background/vpncontroller/states.js";
Expand Down Expand Up @@ -59,10 +60,11 @@ export class BrowserActionPopup extends LitElement {
super();
this.pageURL = null;
this._siteContext = null;
vpnController.state.subscribe((s) => {
/** @type {VPNState} */
this.vpnState = s;
});
/** @type {VPNState} */
this.vpnState = null;
browser.tabs.onUpdated.addListener(() => this.updatePage());
browser.tabs.onActivated.addListener(() => this.updatePage());
vpnController.state.subscribe((s) => (this.vpnState = s));
vpnController.servers.subscribe((s) => (this.servers = s));
proxyHandler.siteContexts.subscribe((s) => {
this._siteContexts = s;
Expand Down Expand Up @@ -207,30 +209,43 @@ export class BrowserActionPopup extends LitElement {
);
}

openServerList() {
const onSelectServerResult = (event) => {
this.stackView.value?.pop().then(() => {
this.requestUpdate();
});
const city = event?.detail?.city;
const country = event?.detail?.country;
if (city) {
const newSiteContext = new SiteContext({
cityCode: city.code,
countryCode: country.code,
origin: this.pageURL,
excluded: false,
});
this.currentSiteContext = newSiteContext;
}
};
async openServerList() {
let serverListResult = Promise.withResolvers();

const ctx = this._siteContext;
const serverListSelectedCity = Utils.getCity(
ctx?.countryCode,
ctx?.cityCode,
this.servers
);
console.log(serverListSelectedCity);
const serverListElement = BrowserActionPopup.createServerList(
serverListSelectedCity,
this.servers,
onSelectServerResult
(e) => serverListResult.resolve(e.detail)
);
this.stackView.value?.push(serverListElement).then(() => {
// Push onto the Stackview and request an update as the
// we read the stackview's top for the titlebar
await this.stackView.value?.push(serverListElement);
this.requestUpdate();

const { city, country } = await serverListResult.promise;
// We're done with the server list, pop it off the stackview
this.stackView.value?.pop().then(() => {
this.requestUpdate();
});
if (!city) {
// Remove the site context
this.currentSiteContext = null;
return;
}
const newSiteContext = new SiteContext({
cityCode: city.code,
countryCode: country.code,
origin: this.pageURL,
excluded: false,
});
this.currentSiteContext = newSiteContext;
}

static sitePreferencesTemplate(
Expand Down Expand Up @@ -304,19 +319,23 @@ export class BrowserActionPopup extends LitElement {
></mz-iconlink>`;
}
/**
*
* @param {ServerCity?} currentCity
* @param { import("./../../components/serverlist.js").ServerCountryList } list - The ServerList to Render
* @param {($0: Event)=>{} } onResult - A callback fired when a city or "back" is clicked.
* - Contains an HTML Event with Optinal
* - { detail.city : ServerCity }
* @returns {HTMLElement} - An already rendered Element, ready to put into the DOM
*/
static createServerList(list = [], onResult = () => {}) {
static createServerList(currentCity = null, list = [], onResult = () => {}) {
const viewElement = document.createElement("section");
viewElement.dataset.title = "Select Location";
render(
html`
<server-list .serverList=${list} @selectedCityChanged=${onResult}>
<server-list
.selectedCity=${currentCity}
.serverList=${list}
@selectedCityChanged=${onResult}
>
</server-list>
`,
viewElement
Expand Down
66 changes: 64 additions & 2 deletions tests/jest/components/serverlist.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ describe("Serverlist Templates", () => {
document.body.append(element);
// Wait for lit to render to the dom
await element.requestUpdate();
const button = element.shadowRoot.querySelector("input[type='radio']");
const button = element.shadowRoot.querySelector(".server-radio-btn");
expect(button.dataset.cityName).not.toBeNull();
button.click();

Expand All @@ -259,7 +259,7 @@ describe("Serverlist Templates", () => {
document.body.append(element);
// Wait for lit to render to the dom
await element.requestUpdate();
const button = element.shadowRoot.querySelector("input[type='radio']");
const button = element.shadowRoot.querySelector(".server-radio-btn");
expect(button.dataset.cityName).not.toBeNull();
button.click();

Expand All @@ -271,6 +271,68 @@ describe("Serverlist Templates", () => {
expect(isOk).toBe(true);
});

test("Serverlist element emits a *{city:null}* if default city is selected", async () => {
/** @type {ServerList} */
const element = document.createElement("server-list");
const newCityEmitted = new Promise((res) => {
element.addEventListener("selectedCityChanged", (e) => {
expect(e.detail.city).toBeNull();
res(true);
});
});
element.serverList = testServerList;
// Set the active city to the one we will click
element.selectedCity = testServerList[0].cities[0];
document.body.append(element);
// Wait for lit to render to the dom
await element.requestUpdate();
const button = element.shadowRoot.querySelector(".default-location-btn");
button.click();

const isOk = await Promise.race([
newCityEmitted,
// Queue a microtask to make sure all events have been processed
new Promise((res) => queueMicrotask(() => res(true))),
]);
expect(isOk).toBe(true);
});

test("Serverlist element selects 'default city' if none is selected", async () => {
/** @type {ServerList} */
const element = document.createElement("server-list");
element.serverList = testServerList;
element.selectedCity = null;
document.body.append(element);
// Wait for lit to render to the dom
await element.requestUpdate();
const button = element.shadowRoot.querySelector(".default-location-btn");
expect(button.checked).toBe(true);
});
test("Serverlist element ignores clicks on 'default city' if none is already selected", async () => {
/** @type {ServerList} */
const element = document.createElement("server-list");
element.serverList = testServerList;
element.selectedCity = null;
document.body.append(element);
// Wait for lit to render to the dom
await element.requestUpdate();
const button = element.shadowRoot.querySelector(".default-location-btn");
expect(button.checked).toBe(true);

const newCityEmitted = new Promise((_, err) => {
element.addEventListener("selectedCityChanged", (e) => {
err(e.detail.city.name);
});
});
button.click();
const isOk = await Promise.race([
newCityEmitted,
// Queue a microtask to make sure all events have been processed
new Promise((res) => queueMicrotask(() => res(true))),
]);
expect(isOk).toBe(true);
});

test("Serverlist can filter by text input", async () => {
/** @type {ServerList} */
const element = document.createElement("server-list");
Expand Down

0 comments on commit 200f896

Please sign in to comment.