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

Handle load errors when config is not initialized #26

Merged
merged 5 commits into from
May 14, 2016
Merged

Handle load errors when config is not initialized #26

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a95eb19
guard against empty columns settings
May 14, 2016
8fb912c
guard against empty epics settings
May 14, 2016
cdaf0a2
Set the columns and epics to hide on install
May 14, 2016
0dd1a2b
Revert "guard against empty columns settings"
May 14, 2016
d1159cf
Revert "guard against empty epics settings"
May 14, 2016
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
4 changes: 4 additions & 0 deletions app/scripts.babel/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

chrome.runtime.onInstalled.addListener(details => {
console.log('previousVersion', details.previousVersion);
chrome.storage.sync.set({
columnsToHide: '137, 1057, 1063',
epicsToHide: '213682'
});
});

chrome.tabs.onUpdated.addListener(tabId => {
Expand Down
27 changes: 15 additions & 12 deletions app/scripts.babel/contentscript.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,12 @@ function toggle_columns() {

function hide_columns() {
chrome.storage.sync.get({ 'columnsToHide': '' }, function(items) {
let column_collection = items.columnsToHide.split(',')
.map(function(i) { return i.trim(); });
let css = generate_column_hiding_css(column_collection);
window.v1_column_hiding_style_el = add_css_to_document(css);
if (typeof items.columnsToHide != 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

If my understanding of chrome's storage API is correct, the { 'columnsToHide': '' } bit should ensure that items.columnsToHide is always defined inside the callback function (either as whatever was stored or as the fallback default of empty string), so I don't think the undefined check is needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BaseCase Agreed. Reverted those changes.

Also, you should try the version on the Chrome Web Store: https://chrome.google.com/webstore/detail/versionone-shot/ifljgacfkdomobnaeibimidndmblihoh

let column_collection = items.columnsToHide.split(',')
.map(function(i) { return i.trim(); });
let css = generate_column_hiding_css(column_collection);
window.v1_column_hiding_style_el = add_css_to_document(css);
}
});
}

Expand Down Expand Up @@ -127,14 +129,15 @@ function toggle_epic_cards() {

function hide_epics() {
chrome.storage.sync.get('epicsToHide', function(items) {
let epic_collection = items.epicsToHide.split(',');

window.v1hiddenEpics = [];

epic_collection.forEach(function(number) {
number = number.trim();
hide_epic(`Epic:${number}`);
});
if (typeof items.epicsToHide != 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to comment above, rather than checking for undefined here, I think it's better to make the first argument to .get be an object that gives a fallback value of empty string for epicsToHide.

let epic_collection = items.epicsToHide.split(',');
window.v1hiddenEpics = [];

epic_collection.forEach(function(number) {
number = number.trim();
hide_epic(`Epic:${number}`);
});
}
});
}

Expand Down