Skip to content
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.

Sign up for GitHub

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

Jump to bottom

MainView materialfx #190

Open
wants to merge 72 commits into
base: filter-panel
Choose a base branch
from
Open

MainView materialfx #190

wants to merge 72 commits into from

Conversation

Anastasia-Nikitina
Copy link
Collaborator

No description provided.

@Anastasia-Nikitina Anastasia-Nikitina marked this pull request as draft April 28, 2023 20:56
@Anastasia-Nikitina Anastasia-Nikitina force-pushed the side-panels-materialfx branch 2 times, most recently from eed3d14 to 79a2acb Compare May 8, 2023 22:13
@Anastasia-Nikitina Anastasia-Nikitina force-pushed the main-view-materialfx branch 2 times, most recently from 1f957a5 to c596cb1 Compare May 21, 2023 00:10
@Anastasia-Nikitina Anastasia-Nikitina changed the base branch from side-panels-materialfx to filter-panel May 21, 2023 18:05
@Anastasia-Nikitina Anastasia-Nikitina marked this pull request as ready for review May 23, 2023 09:29
@Alexander-Ploskin
Copy link
Collaborator

Alexander-Ploskin commented May 25, 2023 •
edited
Loading

  1. Use a small logo for icons, otherwise it is not visible.
    image
    image

  2. Left panel is jerking.
    P3iVVQBthl
    E3RhOmLxiO

  3. Trimmed text.
    image

  4. Outdated placeholder for empty selection.
    image

  5. Wrong position for layer settings popup.
    AUCVdHKwHB

  6. Checkbox doesn't switch when I click on element outside checkbox itself.
    sZN5TEpgmf

  7. Select all checkbox doesn't work at all.
    vUEG7SJXbB

  8. Image view checkboxes hidden from the start and out of sync with files checkboxes.
    rVg7gsnCQa

  9. Wrong position for "One color" checkbox.
    image

  10. Wrong left margin.
    image

  11. No selection style from the start and wrong corner radius.
    image

  12. "Cancel" button doesn't cancel anything, rename it.
    4R0tyxw6fB

  13. "Open importer" button doesn't drop shadow.
    image

  14. Conflict checkbox styles.
    image

  15. Too much empty space.
    image

  16. No Shift key to select range of frames.
    kXCNgBTJJQ

  17. Wrong styles:
    May 25, 2023 1:57:29 PM javafx.css.CssParser declaration WARNING: CSS Error parsing '*{-fx-font-style: 'Roboto Condensed'; -fx-font-weight: 700; -fx-background-color: #78909C; -fx-text-fill: #FFFFFF;}: Expected '<font-style>' while parsing '-fx-font-style' at [1,18] May 25, 2023 1:58:55 PM javafx.css.CssParser declaration WARNING: CSS Error parsing '*{-fx-font-style: 'Roboto Condensed'; -fx-font-size: 14px; -fx-text-fill: #000000}: Expected '<font-style>' while parsing '-fx-font-style' at [1,18] May 25, 2023 1:58:55 PM javafx.css.CssParser term WARNING: CSS Error parsing css://solve.styles.SettingsDialogStylesheet: Unexpected token ';' at [27,26]

  18. Weird.
    ByiCXh5uJJ

  19. Too big fonts.
    image

  20. Too big font.
    image

  21. Non-material marker.
    Xsusd9mGHf

  22. Wrong text style for placeholders.
    image

  23. Wrong border style.
    image

  24. Strange lines.
    image

Alexander-Ploskin
Alexander-Ploskin requested changes Jun 9, 2023
View reviewed changes
src/main/kotlin/solve/catalogue/view/CatalogueSettingsView.kt Outdated
style = "-fx-font-family: ${Style.FontCondensed}; -fx-font-weight: ${Style.FontWeightBold};"
}

var segmentedButton = SegmentedButton(fileNameViewRadioButton, imagePreviewRadioButton)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid public fields

src/test/kotlin/solve/unit/importer/ImporterTests.kt Outdated
Comment on lines 79 to 86
const val pathTestProject = "testData/TestProject1"
const val image1 = "123456789"
const val image2 = "987654321"
const val layer = "alg1_keypoint"
const val pathImage1 = "testData/TestProject1/images/123456789.jpg"
const val pathImage2 = "testData/TestProject1/images/987654321.jpg"
const val pathOutput1 = "testData/TestProject1/alg1_keypoint/123456789.csv"
const val pathOutput2 = "testData/TestProject1/alg1_keypoint/987654321.csv"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name from capital letter

src/main/resources/icons/logo.png Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid text in small logos, it will be better to use something like that
image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, make background transparent

src/main/kotlin/solve/utils/MFXUtils.kt
@@ -0,0 +1,76 @@
package solve.utils
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicates solve.utils.materialfx.MFXNodesUtils

src/main/kotlin/solve/styles/TreeTableViewStylesheet.kt Outdated
@@ -26,18 +19,18 @@ class TreeTableViewStylesheet : Stylesheet() {

treeTableRowCell {
arrow {
backgroundImage += URI("/icons/importer/Shape.png")
backgroundImage += URI("/icons/importer/list_cell_arrow.png")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not svg?

src/main/kotlin/solve/settings/visualization/popover/LayerSettingsPopOverNode.kt Outdated
Comment on lines 27 to 28
dialogClosingController.isClosing.value = true
dialogClosingController.isClosing.value = false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incapsulate in method

src/main/kotlin/solve/settings/visualization/popover/SettingsDialogNode.kt Outdated
import tornadofx.*

class SettingsDialogNode : VBox(10.0) {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space

src/main/kotlin/solve/settings/visualization/popover/PointLayerSettingsPopOverNode.kt
)
addSettingField("One color", buildLandmarkUseOneColorCheckBox(pointLayerSettings), Alignment.Left)
addSettingField(
"",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No title?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more convenient to move the title to the checkbox

src/main/kotlin/solve/settings/visualization/popover/SettingsDialogNode.kt

var title: String = ""

fun addTitle() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why it is not added from the start?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the title contains the name of the markup file. Title is added when the type of landmark is already known

src/main/kotlin/solve/settings/visualization/popover/SettingsDialogNode.kt Outdated

class SettingsDialogNode : VBox(10.0) {

var title: String = ""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public field? Why it is mutable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

field is public because its value is set in another class, when possible define title

Alexander-Ploskin
Alexander-Ploskin requested changes Jun 9, 2023
View reviewed changes
src/main/kotlin/solve/catalogue/view/CatalogueSettingsView.kt Outdated
hbox(130) {
addStylesheet(CatalogueViewStylesheet::class)
maxWidth = 500.0
padding = Insets(0.0, 0.0, 0.0, 0.0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use paddingall

@Alexander-Ploskin
Copy link
Collaborator

Problems:

  1. Placeholder text is trimmed from the start.
    image

  2. Wrong layout.
    b2dSQVnPQZ

  3. Slider label is hidden from the start.
    ujTu2hMkJr

  4. Filters is not shown.
    Xu5Rn8xLGU

  5. Invalid scrollbar position on resizing.
    86mPteRdns

  6. Catalogue tab button loses selected state on second click.
    tXGltK7TNx

  7. Wrong appearance on layer drag-drop.
    zu3W7vwFLE

  8. Apply button and filter panel is lost after no frames was applied.
    gwf29I7roC

@Anastasia-Nikitina Anastasia-Nikitina linked an issue Jun 18, 2023 that may be closed by this pull request
Problems of new design #211
Open
@Anastasia-Nikitina
Copy link
Collaborator Author

comment 1, point 6. It is embedded in the MFXCheckList, also works in demo

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

@Alexander-Ploskin Alexander-Ploskin Alexander-Ploskin requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problems of new design
3 participants
@Anastasia-Nikitina @Alexander-Ploskin @mi-sts