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

Ability to reset widgets layout arrangement to default setting #1794

Closed
keshav2010 opened this Issue Oct 21, 2017 · 12 comments

Comments

Projects
None yet
2 participants
@keshav2010
Contributor

keshav2010 commented Oct 21, 2017

I'm not sure if this feature currently exists or not, I tried to explore tiled but couldn't figure out where is the option that can rearrange everything to default settings that tiled comes with,

I recently moved few on-screen widgets ( mini-map, etc) and now whenever I run tiled again, im not getting back the initial layout arrangement and neither there seem to be any option to do it.

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Oct 23, 2017

Owner

Yeah, with the flexibility to arrange the window however you want should really come a way of resetting things back to default. Currently, the only way to do this is to remove either the whole settings file or the particular keys used to store the window layout.

I'm not actually sure what's the best way to implement this, but the function is provided by Qt Creator for its Designer and Debug modes, and here's one of those implementations:

https://code.woboq.org/qt5/qt-creator/src/plugins/designer/editorwidget.cpp.html#_ZN8Designer8Internal12EditorWidget20resetToDefaultLayoutEv

So, they remove all dock widgets and then re-add them. The function is written in such a way that it is also used for the initial setup, so the code isn't duplicated.

Owner

bjorn commented Oct 23, 2017

Yeah, with the flexibility to arrange the window however you want should really come a way of resetting things back to default. Currently, the only way to do this is to remove either the whole settings file or the particular keys used to store the window layout.

I'm not actually sure what's the best way to implement this, but the function is provided by Qt Creator for its Designer and Debug modes, and here's one of those implementations:

https://code.woboq.org/qt5/qt-creator/src/plugins/designer/editorwidget.cpp.html#_ZN8Designer8Internal12EditorWidget20resetToDefaultLayoutEv

So, they remove all dock widgets and then re-add them. The function is written in such a way that it is also used for the initial setup, so the code isn't duplicated.

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Oct 23, 2017

Owner

@keshav2010 Is this something you'd be interested to have a go at?

Owner

bjorn commented Oct 23, 2017

@keshav2010 Is this something you'd be interested to have a go at?

@keshav2010

This comment has been minimized.

Show comment
Hide comment
@keshav2010

keshav2010 Oct 23, 2017

Contributor

@bjorn definitely I would be working on this feature, this might be a stupid question, I explored the source code a bit and in mainwindow.cpp i found a function void MainWindow::writeSettings() and void MainWindow::readSettings(), i wonder if there might be something to my interest ? I'm planning to delete particular keys as you mentioned before from settings file, and that changes might take effect upon restarting the program if this is possible?

Apart from this, I would be visiting the link to seek other solution also.

Contributor

keshav2010 commented Oct 23, 2017

@bjorn definitely I would be working on this feature, this might be a stupid question, I explored the source code a bit and in mainwindow.cpp i found a function void MainWindow::writeSettings() and void MainWindow::readSettings(), i wonder if there might be something to my interest ? I'm planning to delete particular keys as you mentioned before from settings file, and that changes might take effect upon restarting the program if this is possible?

Apart from this, I would be visiting the link to seek other solution also.

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Oct 23, 2017

Owner

i wonder if there might be something to my interest ? I'm planning to delete particular keys as you mentioned before from settings file, and that changes might take effect upon restarting the program if this is possible?

A reset of the layout should apply immediately, which is possible with the solution I linked to. Of course it is also possible to delete the keys storing the layout from the settings on shutdown and request the user to restart, but I don't see a good reason for implementing it in such a way.

In case you were wondering where the settings are stored, so that you could manually remove related settings keys, I've just added a User Preferences page to the documentation covering this. You would want to delete the MapEditor/State key.

Owner

bjorn commented Oct 23, 2017

i wonder if there might be something to my interest ? I'm planning to delete particular keys as you mentioned before from settings file, and that changes might take effect upon restarting the program if this is possible?

A reset of the layout should apply immediately, which is possible with the solution I linked to. Of course it is also possible to delete the keys storing the layout from the settings on shutdown and request the user to restart, but I don't see a good reason for implementing it in such a way.

In case you were wondering where the settings are stored, so that you could manually remove related settings keys, I've just added a User Preferences page to the documentation covering this. You would want to delete the MapEditor/State key.

@keshav2010

This comment has been minimized.

Show comment
Hide comment
@keshav2010

keshav2010 Oct 23, 2017

Contributor

Yea, that was helpful and I was able to get back to the default layout by deleting that key from the registry, I would be now working upon implementing this in tiled, is this acceptable to include an option under View tab " View > Default Layout", which when pressed resets the widgets layout?

Contributor

keshav2010 commented Oct 23, 2017

Yea, that was helpful and I was able to get back to the default layout by deleting that key from the registry, I would be now working upon implementing this in tiled, is this acceptable to include an option under View tab " View > Default Layout", which when pressed resets the widgets layout?

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Oct 23, 2017

Owner

is this acceptable to include an option under View tab " View > Default Layout", which when pressed resets the widgets layout?

I guess it could be View > Views and Toolbars > Reset to Default Layout. Can be added to the bottom of that menu with a separator before it. See MainWindow::updateViewsAndToolbarsMenu.

Owner

bjorn commented Oct 23, 2017

is this acceptable to include an option under View tab " View > Default Layout", which when pressed resets the widgets layout?

I guess it could be View > Views and Toolbars > Reset to Default Layout. Can be added to the bottom of that menu with a separator before it. See MainWindow::updateViewsAndToolbarsMenu.

@keshav2010

This comment has been minimized.

Show comment
Hide comment
@keshav2010

keshav2010 Oct 24, 2017

Contributor

Yes, that seems better. I've added it in View> Views and Toolbars > Reset to Default Layout
Would be now working on to solve the actual issue, I have exams going on till 31/10/2017, hopefully, today I did manage to put few hours to understand source code but now at least I know what im doing.

Contributor

keshav2010 commented Oct 24, 2017

Yes, that seems better. I've added it in View> Views and Toolbars > Reset to Default Layout
Would be now working on to solve the actual issue, I have exams going on till 31/10/2017, hopefully, today I did manage to put few hours to understand source code but now at least I know what im doing.

@keshav2010

This comment has been minimized.

Show comment
Hide comment
@keshav2010

keshav2010 Oct 29, 2017

Contributor

I've added it and its working now, except this solution requires a restart, however as an added check I've added a popup Dialog box that informs the user about the action that alerts them to save their progress before clicking on this button.
I will be however now working on the solution you linked to earlier, regarding re-adding the widgets that will eventually make this action much more better 👍

Contributor

keshav2010 commented Oct 29, 2017

I've added it and its working now, except this solution requires a restart, however as an added check I've added a popup Dialog box that informs the user about the action that alerts them to save their progress before clicking on this button.
I will be however now working on the solution you linked to earlier, regarding re-adding the widgets that will eventually make this action much more better 👍

@keshav2010

This comment has been minimized.

Show comment
Hide comment
@keshav2010

keshav2010 Nov 1, 2017

Contributor

The alternate approach is now implemented successfully, except I have few implementation doubt, the code I have written right now works in a way that I believe might need to be changed if you decide to add more Widgets in program in future, I have solution for this issue too but yet im just trying to take your opinion before opening a pull request.

So, at the moment, I just refer to mapEditor and extract QList containing DockWidgets and Toolbars, now this QList holds dockWidgets at particular index, this index is fixed for each DockWidget, so I refer to each DockWidget something like this (just an example, do not worry about the variable Names)
dockList.at(0)->show(); (where index 0 holds say, propertiesDock)

but I believe this isn't a good way so instead, I think having a QMap instead to hold the required dockWidget would be better.

Another thing I noticed is something with naming convention that is being followed,
for few dock widgets, the ObjectName begins with small alphabets, for example, propertiesDock, miniMapDock, layerDock, however, few DockWidget's objectName begins with capital letters, like
ObjectsDock (instead of objectsDock). I wonder if there is any reason for this since mostly variable Names begin with small letters?

Contributor

keshav2010 commented Nov 1, 2017

The alternate approach is now implemented successfully, except I have few implementation doubt, the code I have written right now works in a way that I believe might need to be changed if you decide to add more Widgets in program in future, I have solution for this issue too but yet im just trying to take your opinion before opening a pull request.

So, at the moment, I just refer to mapEditor and extract QList containing DockWidgets and Toolbars, now this QList holds dockWidgets at particular index, this index is fixed for each DockWidget, so I refer to each DockWidget something like this (just an example, do not worry about the variable Names)
dockList.at(0)->show(); (where index 0 holds say, propertiesDock)

but I believe this isn't a good way so instead, I think having a QMap instead to hold the required dockWidget would be better.

Another thing I noticed is something with naming convention that is being followed,
for few dock widgets, the ObjectName begins with small alphabets, for example, propertiesDock, miniMapDock, layerDock, however, few DockWidget's objectName begins with capital letters, like
ObjectsDock (instead of objectsDock). I wonder if there is any reason for this since mostly variable Names begin with small letters?

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Nov 1, 2017

Owner

but I believe this isn't a good way so instead, I think having a QMap instead to hold the required dockWidget would be better.

It doesn't sound like a good way indeed. I'd rather expect that you could re-use the same code that is currently setting up the dock widgets initially. So of course the dock widgets aren't accessible in MainWindow, but why should it be? You could instead call a new resetLayout function on the MapEditor.

Another thing I noticed is something with naming convention that is being followed,
for few dock widgets, the ObjectName begins with small alphabets, for example, propertiesDock, miniMapDock, layerDock, however, few DockWidget's objectName begins with capital letters, like
ObjectsDock (instead of objectsDock). I wonder if there is any reason for this since mostly variable Names begin with small letters?

That sounds like an unfortunately inconsistency, but it's not really worth fixing since it would affect users in that their layouts wouldn't restore correctly.

Owner

bjorn commented Nov 1, 2017

but I believe this isn't a good way so instead, I think having a QMap instead to hold the required dockWidget would be better.

It doesn't sound like a good way indeed. I'd rather expect that you could re-use the same code that is currently setting up the dock widgets initially. So of course the dock widgets aren't accessible in MainWindow, but why should it be? You could instead call a new resetLayout function on the MapEditor.

Another thing I noticed is something with naming convention that is being followed,
for few dock widgets, the ObjectName begins with small alphabets, for example, propertiesDock, miniMapDock, layerDock, however, few DockWidget's objectName begins with capital letters, like
ObjectsDock (instead of objectsDock). I wonder if there is any reason for this since mostly variable Names begin with small letters?

That sounds like an unfortunately inconsistency, but it's not really worth fixing since it would affect users in that their layouts wouldn't restore correctly.

@keshav2010

This comment has been minimized.

Show comment
Hide comment
@keshav2010

keshav2010 Nov 2, 2017

Contributor

Alright, that sounds better. I've implemented the feature and also performed the check for various possibilities, for example, i checked "ClearView" action before resetting layout and it was causing some issue with layout placement (but its solved now).
Its neither using QMap nor QList and I've implemented it just as you suggested, by calling a function resetLayout() which I defined inside MapEditor.

Contributor

keshav2010 commented Nov 2, 2017

Alright, that sounds better. I've implemented the feature and also performed the check for various possibilities, for example, i checked "ClearView" action before resetting layout and it was causing some issue with layout placement (but its solved now).
Its neither using QMap nor QList and I've implemented it just as you suggested, by calling a function resetLayout() which I defined inside MapEditor.

@keshav2010

This comment has been minimized.

Show comment
Hide comment
@keshav2010

keshav2010 Nov 13, 2017

Contributor

Okay. I've opened the pull request ( #1809 ), I would be waiting for your response, whether it needs any changes or it can be merged so that I can continue exploring more features, bugs and dive deeper into the code base. 👍

Contributor

keshav2010 commented Nov 13, 2017

Okay. I've opened the pull request ( #1809 ), I would be waiting for your response, whether it needs any changes or it can be merged so that I can continue exploring more features, bugs and dive deeper into the code base. 👍

@bjorn bjorn closed this in b00a46d Dec 20, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment