Skip to content

Modularize settings tab#194

Merged
jwildfire merged 21 commits intodev-v0.9.0from
modularize_settings_tab
Feb 25, 2019
Merged

Modularize settings tab#194
jwildfire merged 21 commits intodev-v0.9.0from
modularize_settings_tab

Conversation

@bzkrouse
Copy link
Copy Markdown
Contributor

@bzkrouse bzkrouse commented Feb 22, 2019

@bzkrouse bzkrouse requested review from jwildfire and pburnsdata and removed request for jwildfire February 22, 2019 19:43
Copy link
Copy Markdown
Contributor

@jwildfire jwildfire left a comment

Choose a reason for hiding this comment

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

Looking good!

msg <- paste0("msg_", key)

### get metadata for the input
setting_key <- as.list(strsplit(key,"\\-\\-"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could use %>% textKeysToList() here.

}


### get the choices for the selectors
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is ok probably for now, but it might be cleaner to refactor to deal with the options and defaults separately at some point. Basically first set the options, and then select the default value (if any).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jwildfire good idea!


fluidRow(
column(6,
column(4,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will be needed soon, but I'm thinking we should comment this out for the v0.9.0 release, and then add it back in once we have a 2nd chart.

@jwildfire jwildfire merged commit e33a16f into dev-v0.9.0 Feb 25, 2019
@jwildfire jwildfire deleted the modularize_settings_tab branch March 18, 2019 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants