This repository has been archived by the owner on May 7, 2020. It is now read-only.
introduced "Default" widget type for sitemaps that is dynamically resolved #1535
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I am not familiar with that code, but why to the default widget be resolved all the time if the instance is already a default one? |
| return result; | ||
| } | ||
|
|
||
| private Widget resolveDefault(Widget widget) { |
There was a problem hiding this comment.
Wouldn't the code be simpler (less ifs above) if we
return widget;if it is not an instance of Default- change the method so you can give the fallback:
private Widget resolveDefault(Widget widget, Widget fallback)andreturn fallback;at the end (so you can use resolveDefault(widget, null) or resolveDefault(widget, widget)
There was a problem hiding this comment.
updated the code according to your suggestions. It looks only slightly better, though ;-)
|
The feature allows the SitemapProvider to simply say "use default widget type of this item" and the framework (uiregistry) then resolves the widget type dynamically (the uis then expect a concrete type). |
| } catch (NumberFormatException e) { | ||
| // no valid number, so the requested page id does not exist | ||
| } | ||
| } | ||
| if (w instanceof Default) { |
…olved Signed-off-by: Kai Kreuzer <[email protected]>
|
Ok, new try - does this look better to you? |
|
👍 😉 |
|
Uff, thanks ;-) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This partially addresses #635 as it is now possible to simply use a "Default" widget with a Player item, so that the switch buttons are rendered.
Signed-off-by: Kai Kreuzer [email protected]