Make the popup a prettier #4
Conversation
| <li><a href="#" data-lang data-display-lang="English">English (Default)</a></li> | ||
| <li><a href="#" data-lang="fr" data-display-lang="Français">Français</a></li> | ||
| <li><a href="#" data-lang="de-DE" data-display-lang="Deutsch">Deutsch (Deutschland)</a></li> | ||
| <li><a href="#" data-lang="ja" data-display-lang="日本語">日本語</a></li> |
There was a problem hiding this comment.
I think we can avoid complexity by getting rid of data-display-lang in favor of the inner value. That way the displayed language will always be what the user actually clicked.
| document.querySelectorAll('a[data-lang]').forEach(function(element) { | ||
| element.addEventListener('click', function(event) { | ||
| chrome.storage.sync.set({'language': event.target.getAttribute('data-lang')}, function() { | ||
| chrome.storage.sync.set({'language': event.target.getAttribute('data-lang'), 'displaylang': event.target.getAttribute('data-display-lang')}, function() { |
There was a problem hiding this comment.
I don't think it makes sense for us to keep track of both values. They're totally related, and knowing one should be enough for us to know both.
|
For the language list, one thing we could do is define it in javascript, something like; <script>
let languages = {
'en': 'English (Default)'
'fr': 'Français',
...
}
for (var lang of languages) {
var node = document.createElement('li');
node.setAttribute('data-lang', lang);
node.appendChild(document.createTextNode(languages[lang]));
document.querySelector('[data-languages]').appendChild(node);
};
</script>
...
<ul data-languages>
</ul>Don't copy/paste this as I'm sure it doesn't work out of the box, but you get the idea. That way, when a link is clicked, we get access to the |
|
Another suggestion would be to split this PR in two, as you're introducing two changes; 1) new style and 2) new displayed text value. It will be easier for us to review each change individually. |
2 participants
Added styling to language selector popup
Changed the tag used to display Current selected language