Changes for discussion #36
Conversation
|
|
||
| @Outlet | ||
| private JMenuItem saveItem; | ||
| // --- UI Components (Injected by Sierra) --- |
There was a problem hiding this comment.
Current convention is to declare outlets as follows, although obviously either will work.
Initializing the value to null avoids a warning in Intellij.
| @Outlet | ||
| private JSplitPane splitPane; | ||
|
|
||
| @Outlet |
There was a problem hiding this comment.
This outlet is not needed when the editor pane is declared in markup - see below.
| * business logic. | ||
| */ | ||
| public class MainFrame extends JFrame { | ||
|
|
There was a problem hiding this comment.
Unless you plan to serialize the frame instance, this declaration isn't strictly necessary.
| private JLabel filePathLabel; // The <label> for the file path | ||
|
|
||
| // --- Manually Created Components --- | ||
| private RSyntaxTextArea editorPane; |
| private RSyntaxTextArea editorPane; | ||
|
|
||
| public MainFrame() { | ||
| super("Sierra UI Previewer"); |
There was a problem hiding this comment.
Minor - this is not needed.
| public class SierraPreviewerApp { | ||
|
|
||
| public static void main(String[] args) { | ||
| FlatLightLaf.setup(); |
There was a problem hiding this comment.
This is the recommended way to initialize FlatLaf.
| // --- Manually Created Components --- | ||
| private RSyntaxTextArea editorPane; | ||
| @Outlet | ||
| private RSyntaxTextArea editorPane; |
There was a problem hiding this comment.
Existing Sierra code uses spaces rather than tabs.
| setAutoActivationRules(true, "< "); | ||
| } | ||
|
|
||
| private void loadTags() { |
There was a problem hiding this comment.
Minor simplification that avoids unnecessary local declarations.
| <label name="filePathLabel" text="" padding="4, 6, 4, 6"/> | ||
| <split-pane name="splitPane" resizeWeight="0.5" weight="1"> | ||
| <scroll-pane weight="1"> | ||
| <syntax-text-area name="editorPane" columns="80" rows="25" |
There was a problem hiding this comment.
Declare editor in markup.
There was a problem hiding this comment.
I just realized that this will make it seem like "syntax-text-area" is part of the standard distribution, which it is not. So maybe this is not a good idea after all.
There was a problem hiding this comment.
Yeah agree - only valid widgets should be in the xml - even if they are placeholders to be replaced at runtime
| codeFoldingEnabled="true" | ||
| antiAliasingEnabled="true"/> | ||
| </scroll-pane> | ||
| <column-panel name="previewPanel" padding="5, 5, 5, 5" weight="1"/> |
There was a problem hiding this comment.
Drop placeholder label that didn't seem to be visible in UI.
There was a problem hiding this comment.
yeah at one point I had it showing and then somehow messed it up
| // Map to store the final resolved attributes for each tag | ||
| // TagName -> {AttributeName -> Description/ValueDefinitionString} | ||
| private final Map<String, Map<String, String>> elementAttributeDefinitions = new HashMap<>(); | ||
There was a problem hiding this comment.
Trailing whitespace is trimmed.
lawson89
left a comment
lawson89
left a comment
There was a problem hiding this comment.
Greg - I am good with all these changes!
No description provided.