Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Address TODO - LanguageBehavior is now offered by the generic PropertiesFromAncestorBehavior so use it #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+87 −91
Open

Address TODO - LanguageBehavior is now offered by the generic PropertiesFromAncestorBehavior so use it #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .version
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v1.0.x
v1.1.x
5 changes: 4 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ Expandable/collapsible summary and details with animation
<template>
<script src="../webcomponentsjs/webcomponents-lite.js"></script>
<link rel="import" href="papyrus-details.html">
<link rel="import" href="./demo/container-with-lang-choice.html">
<style>
papyrus-details { font-family: Roboto; background-color: #ddd; padding: 1em; border-radius: 4px; }
summary { font-size: 120%; font-weight: bold; }
</style>
<next-code-block></next-code-block>
<container-with-lang-choice lang="en" available-languages-expr="Object.getOwnPropertyNames(PapyrusDetails.resources)">
<next-code-block></next-code-block>
</container-with-lang-choice>
</template>
</custom-element-demo>
```
Expand Down
3 changes: 2 additions & 1 deletion bower.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
"description": "Expand and collapse information with summary text displayed at all times",
"main": "papyrus-details.html",
"dependencies": {
"polymer": "Polymer/polymer#^1.4.0"
"polymer": "Polymer/polymer#^1.4.0",
"properties-from-ancestor-behavior": "^1.0"
},
"license": "http://polymer.github.io/LICENSE.txt",
"devDependencies": {
Expand Down
43 changes: 43 additions & 0 deletions demo/container-with-lang-choice.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<link rel="import" href="../../polymer/polymer.html" />
<dom-module id="container-with-lang-choice">
<template>
<label>
Interface Language:
<select id="languageSelect" value="{{lang::input}}"></select>
</label>
<content select="*"></content>
</template>
</dom-module>
<script>
Polymer({
is: "container-with-lang-choice",
properties: {
lang: {
notify: true,
reflectToAttribute: true,
},
availableLanguagesExpr: {
type: String,
observer: '_availableLanguagesExprChanged',
},
availableLanguages: {
type: Array,
observer: '_availableLanguagesChanged',
},
},
_availableLanguagesExprChanged: function (availableLanguagesExpr) {
this.availableLanguages = eval(availableLanguagesExpr);
},
_availableLanguagesChanged: function (availableLanguages) {
// adding options here instead in <template> to suport IE11 - https://github.com/Polymer/polymer/issues/1735

this.$.languageSelect.innerHTML = '';

for (var i = 0; i < availableLanguages.length; ++i) {
var option = document.createElement('option');
option.innerText = availableLanguages[i];
this.$.languageSelect.appendChild(option);
}
},
});
</script>
25 changes: 14 additions & 11 deletions demo/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!doctype html>
<html lang="en">
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, minimum-scale=1, initial-scale=1, user-scalable=yes">
Expand All @@ -10,6 +10,7 @@

<link rel="import" href="../../iron-demo-helpers/demo-pages-shared-styles.html">
<link rel="import" href="../../iron-demo-helpers/demo-snippet.html">
<link rel="import" href="./container-with-lang-choice.html">
<link rel="import" href="../papyrus-details.html">

<style is="custom-style" include="demo-pages-shared-styles">
Expand All @@ -18,15 +19,17 @@
<body>
<div class="vertical-section-container centered">
<h3>Basic papyrus-details demo</h3>
<demo-snippet>
<template>
<papyrus-details>
<summary>Expand this block of text...</summary>
<p>Lorem ipsum dolor sit amet, ad eum brute mundi maiestatis. Augue veniam ne duo. Ex quo intellegam appellantur. Sumo ullum eu vis, vim id everti mandamus. Has ullum movet disputationi cu, mea ei meis nusquam, ne sed virtute albucius repudiandae.</p>
<p>Dicta minim ei has, falli insolens efficiendi ad ius. Tollit reprimique delicatissimi ei nec, duis nobis nam in. Id intellegam ullamcorper mel, malorum veritus ea duo, habemus minimum mandamus est ut. Prima delectus at sea, mollis impedit accumsan mei ad, dico alia commune ex vix. Laoreet reformidans no mel. Ut fastidii omnesque indoctum ius. </p>
</papyrus-details>
</template>
</demo-snippet>
<container-with-lang-choice lang="en" available-languages-expr="Object.getOwnPropertyNames(PapyrusDetails.resources)">
<demo-snippet>
<template>
<papyrus-details>
<summary>Expand this block of text...</summary>
<p>Lorem ipsum dolor sit amet, ad eum brute mundi maiestatis. Augue veniam ne duo. Ex quo intellegam appellantur. Sumo ullum eu vis, vim id everti mandamus. Has ullum movet disputationi cu, mea ei meis nusquam, ne sed virtute albucius repudiandae.</p>
<p>Dicta minim ei has, falli insolens efficiendi ad ius. Tollit reprimique delicatissimi ei nec, duis nobis nam in. Id intellegam ullamcorper mel, malorum veritus ea duo, habemus minimum mandamus est ut. Prima delectus at sea, mollis impedit accumsan mei ad, dico alia commune ex vix. Laoreet reformidans no mel. Ut fastidii omnesque indoctum ius. </p>
</papyrus-details>
</template>
</demo-snippet>
</container-with-lang-choice>
</div>
</body>
</html>
11 changes: 0 additions & 11 deletions language-behavior.html
View file
Open in desktop

This file was deleted.

35 changes: 22 additions & 13 deletions papyrus-details.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<link rel="import" href="../polymer/polymer.html">
<link rel="import" href="language-behavior.html">
<link rel="import" href="../properties-from-ancestor-behavior/properties-from-ancestor-behavior.html">

<dom-module id="papyrus-details">
<template>
Expand All @@ -15,28 +15,38 @@
<div id="summary-container">
<content select="summary"/>
</div>
<a id="toggle" on-tap="toggle" href="javascript:void(0)">[[_getLinkText(expanded,language)]]</a>
<a id="toggle" on-tap="toggle" href="javascript:void(0)">[[_getLinkText(expanded,lang)]]</a>
<div id="expander" class$="[[_getHiddenClass(expanded)]]">
<content/>
</div>
</template>

<script>
(function(){
var languages = {
en: { open: "Open", close: "Close" },
fr: { open: "Ouvrir", close: "Fermer" },
de: { open: "Öffnen", close: "Schließen" },
es: { open: "Abrir", close: "Cerrar" }
};
PapyrusDetails = {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Global variables suck. I can't see a reason to change this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well. Global variables are not verboten. Global variable here is just an easy way to access things can be made available before any element is created, where the usage [requires it]/[would benefit from it]. By correctly representing scopes of variance, they aid in understanding complexity and encapsulating variance - if something exists with intention to be always same for all instances, there's no reason it should pollute the properties of each instance, where a user could break these intentions.

Even Polymer prescribes using global variables, not only for behaviors, but for elements as well, fore example to have a custom constructor.
Similarly, global variables are natural for usages where components are declared as vanilla JS classes, (arguably, where Polymer is heading) since a named class declaration essentially creates a variable of that name. Using ES6 module with the class as 'default export' could save from this, but it's only an option. And with ES6 classes, IMO best way to represent this would be a static get, just like the properties declaration in Polymer 2.0, which also don't get copied to instances.

Here, the reason for the global variable is because usages, such as this <container-with-lang-choice>, benefit from having available the list of supported languages. If we moved PapyrusDetails.resources anywhere else, like on the instance of element, we will need rewrite this pretty simple code to something harder to understand. I'm keen to see how such an implementation would stack up.

PapyrusDetails.resources = {
en: { open: "Open", close: "Close" },
fr: { open: "Ouvrir", close: "Fermer" },
de: { open: "Öffnen", close: "Schließen" },
es: { open: "Abrir", close: "Cerrar" }
};

/**
@demo demo/index.html
*/
Polymer({
is: 'papyrus-details',
behaviors: [ LanguageBehavior ],
properties: { expanded: { type: Boolean, value: false }},
behaviors: [
PropertiesFromAncestorBehavior({
lang: { defaultValue: 'en' },
}),
],
properties: {
expanded: { type: Boolean, value: false },
lang: String,
},
_getHiddenClass: function() { return this.expanded ? "" : "hidden"; },
_getLinkText: function () {
var language = languages[this.language] || languages.en;
var language = PapyrusDetails.resources[this.lang];
return this.expanded ? language.close : language.open;
},
attached: function(){
Expand All @@ -47,6 +57,5 @@
},
toggle: function() { this.set('expanded', !this.expanded); }
});
})();
</script>
</dom-module>
52 changes: 0 additions & 52 deletions test/language-behavior_test.html
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion test/papyrus-details_test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@

test('localises link text', () => {
let el = fixture('basic');
el.set('language', 'fr');
el.lang = 'fr';
assert.equal("Ouvrir", el.getElementsByTagName('a')[0].innerText);
});
});
Expand Down