Skip to content
This repository was archived by the owner on Mar 5, 2024. It is now read-only.

DOCSP-877 & DOCSP-879 - Parameterize filter and update saved pref. #161

Merged
merged 2 commits into from
Nov 3, 2017

Conversation

jdestefano-mongo
Copy link
Contributor

@jdestefano-mongo jdestefano-mongo commented Oct 31, 2017

MUST BE MERGED WITH:
mongodb/docs#3074

<ul class="tab-strip tab-strip--singleton" role="tablist">
{{ for tab in tabs sortLanguages }}
<ul class="tab-strip tab-strip--singleton" role="tablist" PREFERENCE>
{{ for tab in tabs FILTER }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we use some kind of notation with special characters or something to make it less like plain english? For example, $preference

Copy link
Contributor

Choose a reason for hiding this comment

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

I would go with e.g. %PREFERENCE%, yeah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@i80and i80and left a comment

Choose a reason for hiding this comment

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

A couple of suggestions, but largely looks fine!

* @param {object} value The "tabId" and optional "type" (tab set)
*/
set tabPref(value) {
const tabPref = this.tabPref;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd not make this local variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Discussed offline, calling the getter method above.

* @returns {object} Tab preference object.
*/
get tabPref() {
let pref = JSON.parse(window.localStorage.getItem(this.key));
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider making this just

return JSON.parse(window.localStorage.getItem(this.key)) || {};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

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.

2 participants