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.

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

Created a new Preferences window, and added PromptSaveOnExit option. #1216

Merged
merged 5 commits into from
Apr 27, 2018

Conversation

dhood
Copy link
Contributor

@dhood dhood commented Apr 6, 2018

Redo of #1106 on behalf of @mastereric , who wrote:

For tutorials installed to a read-only location, it may be disorienting for the user to be prompted to save. For this reason, I created a preferences dialog window, with a boolean option for whether to Prompt Save on Exit, defaulting to true. These preferences are saved to any config .rviz files that are created.

EliteMasterEric and others added 4 commits March 5, 2018 11:31
@dhood
Copy link
Contributor Author

dhood commented Apr 6, 2018

@mastereric thanks for the work adding the generalised preferences dialog! The PromptSaveOnExit option looks good.

As mentioned in #1106 (comment), the change will break ABI (because it adds a member to the VisualizationFrame class). This means we won't be able to release it into indigo/kinetic/lunar, so will hold it for when we release rviz into melodic.

Since we are breaking ABI anyway, I changed your PR to use a generalised Preferences struct, so that we will be able to add additional preferences in the future without breaking ABI again.

@dhood dhood force-pushed the disable_save_prompt branch from a908728 to 5b6d519 Compare April 6, 2018 20:36
@dhood dhood added this to the first melodic release milestone Apr 6, 2018
@EliteMasterEric
Copy link

@dhood Great idea creating the Preferences struct! I'm not well-versed in what affects the ABI and what causes conflicts in applications.

I'm certainly excited to see my major changes in the upcoming Melodic release!

I'm wondering if you have any opinions/ideas/suggestions for other options that would be useful to include in the Preferences menu.

@dhood
Copy link
Contributor Author

dhood commented Apr 11, 2018

I'm certainly excited to see my major changes in the upcoming Melodic release!

congrats! even though your original PR had to be replaced by this one; you will still be credited in the changelog 😄

I'm wondering if you have any opinions/ideas/suggestions for other options that would be useful to include in the Preferences menu.

Not necessarily, but these things pop up as we/contributors add different functionality; it will be helpful to be able to add options to this struct without breaking ABI for those features (from melodic onwards)

@dhood dhood changed the base branch from kinetic-devel to melodic-devel April 27, 2018 23:02
@dhood dhood merged commit a3290d0 into melodic-devel Apr 27, 2018
@dhood dhood deleted the disable_save_prompt branch April 27, 2018 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants