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

feature/CLAN6-43--widget-person-draw #155

Merged
merged 14 commits into from
Jan 10, 2020
Merged

feature/CLAN6-43--widget-person-draw #155

merged 14 commits into from
Jan 10, 2020

Conversation

jplucinski
Copy link
Contributor

  • Person Draw Widget
  • MultiTextInput.js - multiline dialog input

feature/CLAN6-43--widget-person-draw
9ce1796 
- Person Draw Widget
- MultiTextInput.js - multiline dialog input
szymon-owczarzak
szymon-owczarzak suggested changes Dec 13, 2019
View reviewed changes
Copy link
Contributor

@szymon-owczarzak szymon-owczarzak left a comment

Choose a reason for hiding this comment

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

As discussed please move draw logic to backend so:

  • each browser session will have the same draw resuls
  • draw results will be saved
  • draw results will be available for all new client sessions
  • less WS communication - only when new draw is done

@jplucinski
Copy link
Contributor Author

After catch up with @szymon-owczarzak we decided to add backed sync for widget.

goeson00
goeson00 requested changes Dec 15, 2019
View reviewed changes
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/MultiTextInput.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/types/PersonDrawWidget/index.js Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/types/PersonDrawWidget/index.js Outdated Show resolved Hide resolved
@jplucinski jplucinski added the enhancement New feature or request label Dec 16, 2019
jplucinski added 3 commits December 17, 2019 17:28
feature/CLAN6-43--widget-person-draw
d1455c2 
- moved persistence to backend
- saving state when server restarts
- dialog form validation
- CR fixes
feature/CLAN6-43--widget-person-draw
b76a29d 
- update uuid in form
- fix imports
Merge branch 'master' into feature/CLAN6-43--widget-person-draw
a0bb957 
# Conflicts:
#	cogboard-webapp/src/components/widgets/dialogFields/index.js
szymon-owczarzak
szymon-owczarzak suggested changes Dec 19, 2019
View reviewed changes
Copy link
Contributor

@szymon-owczarzak szymon-owczarzak left a comment

Choose a reason for hiding this comment

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

Please we need JUnit for complex BL like this

cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/PersonDrawWidget.kt Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/types/PersonDrawWidget/index.js Show resolved Hide resolved
cogboard-webapp/src/components/widgets/index.js Outdated Show resolved Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/PersonDrawWidget.kt Outdated Show resolved Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/PersonDrawWidget.kt Outdated Show resolved Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/PersonDrawWidget.kt Outdated Show resolved Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/PersonDrawWidget.kt Outdated Show resolved Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/PersonDrawWidget.kt Outdated Show resolved Hide resolved
cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/PersonDrawWidget.kt Outdated Show resolved Hide resolved
cogboard-webapp/src/components/widgets/dialogFields/index.js Show resolved Hide resolved
jplucinski added 4 commits January 7, 2020 15:09
feature/CLAN6-43--widget-person-draw
a41bcc2 
- extract implementation from PersonDrawWidget.kt
Merge branch 'master' into feature/CLAN6-43--widget-person-draw
6fb9aeb 
# Conflicts:
#	cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/WidgetIndex.kt
#	cogboard-webapp/src/components/widgets/index.js
#	functional/cypress-tests/cypress/config/travis.json
#	functional/cypress-tests/cypress/integration/widgets.js
#	functional/cypress-tests/cypress/support/widgetAssertions.js
feature/CLAN6-43--widget-person-draw
e3f6c05 
- storing widget context in persisted content
- add unit tests for PersonDrawIndexer.kt
feature/CLAN6-43--widget-person-draw
99f60e1 
- storing widget data in persisted content
- util function and unit tests
szymon-owczarzak
szymon-owczarzak suggested changes Jan 8, 2020
View reviewed changes
Copy link
Contributor

@szymon-owczarzak szymon-owczarzak left a comment

Choose a reason for hiding this comment

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

Please revert all this strange format changes

szymon-owczarzak
szymon-owczarzak reviewed Jan 8, 2020
View reviewed changes
cogboard-app/src/main/kotlin/com/cognifide/cogboard/widget/type/persondraw/PersonDrawUtils.kt Outdated
import java.time.ZoneId
import kotlin.random.Random

fun <T> List<T>.getNextListIndex(currentIndex: Int): Int {
Copy link
Contributor

@szymon-owczarzak szymon-owczarzak Jan 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

You could use when expression and ranges here:

fun <T> List<T>.getNextListIndex(currentIndex: Int) = when {
    this.isEmpty() -> -1
    currentIndex in 0..(size - 2) -> currentIndex + 1
    else -> 0
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I will use your version.

szymon-owczarzak
szymon-owczarzak reviewed Jan 8, 2020
View reviewed changes
...ard-app/src/test/kotlin/com/cognifide/cogboard/widget/type/persondraw/PersonDrawUtilsTest.kt
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Test

class PersonDrawUtilsTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice ! i like this

@jplucinski
Copy link
Contributor Author

Changes in cypress scripts formatting are expected - consulted with @staszke

feature/CLAN6-43--widget-person-draw
6c41b99 
- MultiTextInput.js - fix list key
- PersonDrawWidget.kt - read config only ones
@jplucinski jplucinski self-assigned this Jan 10, 2020
@jplucinski jplucinski merged commit 77207b0 into master Jan 10, 2020
@szymon-owczarzak szymon-owczarzak deleted the feature/CLAN6-43--widget-person-draw branch January 17, 2020 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcinkp marcinkp marcinkp left review comments

@szymon-owczarzak szymon-owczarzak szymon-owczarzak approved these changes

@goeson00 goeson00 goeson00 approved these changes

Assignees

@jplucinski jplucinski

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jplucinski @marcinkp @szymon-owczarzak @goeson00