-
Notifications
You must be signed in to change notification settings - Fork 0
Peer review
The installation wizard is very nice, the .exe can be opened afterwards from the installed directory. Everything worked as supposed to.
After configuring the paths and updating the dependencies, the application opened as promised with the suggested command. Some warnings were outputted in the terminal, but nothing critical nor red showed up.
However, the some error messages on the way were pretty cryptic (for example an Execution default-compile of goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile failed: Unsupported major.minor version 59.0
error could be solved by refreshing JDK to a newer version).
Installation test was performed on Windows 10 Home Build 19043. Maybe add some installation tips for MacOS or Linux users?
The code is well structured. Maybe controllers should be further separated into a 'controllers' folder, so 'App' can be a standalone launchable front-end in the ee.ut folder.
Blocks of code are properly formatted and separated. Some methods are very long (for example startTimeline() in App.java and startEdit() in EventsController.java), which makes it hard to read. Maybe some sub-methods can be implemented for better readability? More comments wouldn't hurt and make code easier to understand. More empty lines can also give reader some breathing room.
IntelliJ IDEA tool SpotBugs found 17 bugs from your code of which 15 are 'of concern' and 2 are 'troubling'. From the 'of concern' bugs, many reference 'misuse of static fields' or 'storing reference to mutable object'. Maybe making your window instances extend Singleton class will fix this problem? 'Troubling' bugs are not concerning and irrelevant in the code context of your code, but still interesting to check through (Possible NullPointer dereference in App.java lines 304-309).
Acceptance tests are based on the release notes of iteration 3.
- It is possible to add new events.
- Users can delete existing events. When doing so, the program asks the user if they really want to delete the event which is nice.
- Users can add new events.
- Users can edit existing events.
- Editing the content of the event works well:
- It is possible to choose between a paragraph and different types of headings (1–6).
- Users can edit the font size, make it bold, underline it, strike it through and insert horizontal rules.
- It is also possible to align the text (left, middle, right, justify), add indentations, bulleted and numbered lists.
- Users can add pictures to the event.
- Changing the picture size works when the program is in fullscreen.
- Adding a label to the event works.
- Giving an order number for the event works well - the events are displayed in the order they are marked.
- Users can cancel adding a new event.
- Users can save a new event to the timeline.
- Users can edit existing events.
- Changing the picture size did not work when the program was not fullscreen. When not in fullscreen:
- Sometimes the button for resizing did not appear at all.
- Sometimes the button did appear, but dragging it did nothing.
- Sometimes after deleting the added picture, the resize button still stays there.
- But this bug was mentioned in the release notes for iteration 3.
- When the cursor is left for the first character of the text as shown in the image below, when pushing the left arrow, the tab switches to Settings (Sätted).
- Users can change the label of the event on the timeline.
- Users can change the gap size between the events on the timeline
- Users can choose whether the events on the timeline are packed or not.
- Users can save the changes they made on the settings tab.
- When resizing the program window, the 'Save' button may be left out of the viewable area.
- When opening an existing timeline, the saved settings are not displayed on the settings tab, instead the settings are always the same: Joon, 200 and the packing option unticked.
- It is not clear what the difference between the options 'Joon' and 'Punkt' are. When checking the preview, it looks the same with both options.
- It is also unclear what the option for packing the events does. With 3 events on the timeline with gap 200, the preview looks the same with the option to pack enabled or disabled.
- When opening the preview tab and making changes to the timeline, the changes appear on the preview after they have been saved. This is very nice.
- When pressing the button 'Read more ...' the event is opened in front of the preview.
- The preview tab seems to work very well and no bugs were discovered.
- When opening the program the users can choose to open either a new timeline or an existing timeline.
- Users can save the timeline from 'file' -> 'Salvesta ajajoon'.
- It is also possible to save the timeline when already editing a timeline and closing the program. Before the program is closed, it asks if the user would like to Save and close (Salvesta ja jätka), Close (Jätka) or Cancel (Katkesta).
- If there is an existing timeline, and the user chooses to create a new timeline and save it, it writes over the file where the initial timeline was. However, a similar problem was mentioned under the Known bugs section of the release notes of iteration 3.
- All in all the program is designed to be easy to use and all the actions seem clear.
- Adding some styling to the default style could make the program visually look nicer.
- The timeline.html document created as a result could have a stronger emphasis on the style, maybe add some shadows, borders, background.
- The styling of the close button of the closer view of a timeline item (opened when pressing Read more ...) is a bit weak at the moment.
- When resizing the program window, on some buttons the entire text does not stay visible. For example, Muuda and Kustuta on the Events tab. Since the text on those buttons is very short, the buttons could have a minimal with.
- The placing of the button 'Salvesta' on the Settings tab could be better, right now there is a big gap between the button and the rest of the content. Also, the button should not disappear from view when making the window smaller.
- Most of the language used in the user interface is Estonian, but there are a couple of things in English. For example: file, 'Read more ...' (in the preview tab).
- Capitalization of the names of the options and tabs is inconsistent. For example: file and eelvaade vs Sätted and Sündmused.