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

Fix Bug 1480106, on mobile, show browser name istead of icon #776

Merged
merged 2 commits into from
Aug 30, 2018
Merged

Fix Bug 1480106, on mobile, show browser name istead of icon #776

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2243c58
Fix Bug 1480106, on mobile, show browser name istead of icon
Aug 23, 2018
7b1e806
Change Webview to WebView
Aug 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion macros/CompatBeta.ejs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,42 @@ function writeNotes(support, browserId) {
return output;
}

/**
* Formats the `browserNameKey` for display in the UI
* @param {String} browserNameKey - The browser name
* @returns Correctly formatted browser name
*/
function getFormattedBrowserName(browserNameKey) {
Copy link
Member

Choose a reason for hiding this comment

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

This function is OK for now. In the future, the formatted name should come from the compat data package.
(the export is browsers.browserNameKey.name), but we're missing two browsers still (see mdn/browser-compat-data#2690 and mdn/browser-compat-data#1712)

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes, that will make this much simpler.

let firstChar;
switch(browserNameKey) {
case 'chrome':
case 'edge':
case 'firefox':
case 'opera':
case 'safari':
firstChar = browserNameKey.charAt(0);
return browserNameKey.replace(firstChar, firstChar.toUpperCase());
case 'ie':
return browserNameKey.toUpperCase();
case 'webview_android':
return 'Webview Android';
Copy link
Member

Choose a reason for hiding this comment

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

Webview -> WebView

Copy link
Author

Choose a reason for hiding this comment

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

Updated.

case 'chrome_android':
return 'Chrome Android';
case 'firefox_android':
return 'Firefox Android';
case 'opera_android':
return 'Opera Android';
case 'edge_mobile':
return 'Edge Mobile';
case 'safari_ios':
return 'Safari iOS';
case 'samsunginternet_android':
return 'Samsung Internet Android';
default:
return browserNameKey;
}
}

/*
For a single row, write all the cells that contain support data.
(That is, every cell in the row except the first, which contains
Expand Down Expand Up @@ -488,6 +524,7 @@ function writeCompatCells(supportData) {
supportInfo += getCellString(null);
}

let browserName = getFormattedBrowserName(browserNameKey);
let supportClass = getSupportClass(support);
output += `<td class="bc-supports-${supportClass} bc-browser-${browserNameKey}`;
legendItems.add('support_' + supportClass);
Expand All @@ -496,7 +533,7 @@ function writeCompatCells(supportData) {
output += ' bc-has-history';
}

output += `">${supportInfo}`;
output += `"><span class="bc-browser-name">${browserName}</span>${supportInfo}`;
Copy link
Member

Choose a reason for hiding this comment

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

Having this hidden for now works for me. Note that we want to remove icons and use text everywhere and not just on mobile in the future. See #616. I'm not sure when we want to do this, though. Maybe now isn't the time for it. So this is OK with me 👍

Copy link
Author

Choose a reason for hiding this comment

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

Agreed. The scope of this was mobile specific, as that is where we will mostly see the use case of people turning of custom fonts.


if (needsNotes) {
output += writeCellIcons(support);
Expand Down