Added Expand/Collapse API to GroupLayer- Contd#4379
Added Expand/Collapse API to GroupLayer- Contd#4379Oval17 wants to merge 5 commits intomapeditor:masterfrom
Conversation
Oval17
force-pushed
the
3997_expand_collapse_layers_api
branch
2 times, most recently
from
302046e to
090240f
Compare
|
Kindly review @bjorn @PalaBeaveR ! |
bjorn
left a comment
bjorn
left a comment
There was a problem hiding this comment.
Thanks! This already looks like an improvement over #4009.
Please don't forget to update docs/scripting-doc/index.d.ts with the added API and add a line to the NEWS.md file (consider rebasing first, to avoid conflicts).
There's also still a potential crash (see comment) and I've suggested some further tweaks.
src/tiled/layerdock.h
Outdated
| Q_INVOKABLE bool isExpanded(EditableGroupLayer *layer) const; | ||
| Q_INVOKABLE void setExpanded(EditableGroupLayer *layer, bool expanded); | ||
| bool isExpanded(GroupLayer *layer) const; | ||
| void setExpanded(GroupLayer *layer, bool expanded); |
There was a problem hiding this comment.
I think it might be a good idea to move these extra non-scripting functions into LayerView.
There was a problem hiding this comment.
Done.
Removed isExpanded(GroupLayer*) / setExpanded(GroupLayer*, bool) from LayerDock and added them to LayerView.
src/tiled/objectsdock.h
Outdated
| Q_INVOKABLE bool isExpanded(EditableObjectGroup *layer) const; | ||
| Q_INVOKABLE void setExpanded(EditableObjectGroup *layer, bool expanded); | ||
| bool isExpanded(ObjectGroup *layer) const; | ||
| void setExpanded(ObjectGroup *layer, bool expanded); |
There was a problem hiding this comment.
I think these non-script functions are better moved to ObjectsView.
There was a problem hiding this comment.
Removed isExpanded(ObjectGroup*) / setExpanded(ObjectGroup*, bool) declarations
There is an |
Group layers can now be expanded/collapsed from scripts
…do related changes
- Changed Q_INVOKABLE parameter types from EditableGroupLayer*/EditableObjectGroup* to EditableLayer*/EditableObjectGroup* using asGroupLayer() cast; non-group layers now return false/no-op instead of aborting with a type error - Moved isExpanded/setExpanded(GroupLayer*) helpers from LayerDock to LayerView with mMapDocument null checks to fix potential crash when no map is open - Moved isExpanded/setExpanded(ObjectGroup*) helpers from ObjectsDock to ObjectsView - Fixed brace style for function definitions in objectsdock.cpp - Added missing #include <QCoreApplication> to objectsdock.cpp - Removed spurious #include "map.h" from changelayer.cpp - Added LayersView and ObjectsView interfaces to scripting API (index.d.ts) - Added changelog entry to NEWS.md
Oval17
force-pushed
the
3997_expand_collapse_layers_api
branch
from
74a4f1c to
850a90b
Compare
|
I have Implemented all the suggested Review Comments : |
3 participants
Resolves : #3997
Continuation of PR : #4009
Addressed all the review comments :
Reverted grouplayer.h changess.
Reverted editablegrouplayer.h/cpp and editableobjectgroup.h/cpp , Removed the isExpanded/setExpanded properties and methods that were added directly to the layer object
Moved Q_INVOKABLE isExpanded/setExpanded out of the header files , In
layerdock.handobjectsdock.h, the methods were defined inline. Moved the implementations to layerdock.cpp and objectsdock.cpp respectively.Added null checks , The moved implementations now check for a null layer argument and call ScriptManager::instance().throwError(...) with a proper translated error message, matching the pattern used rest of pplaces.