[siteplan] show red yellow comparism also in non-developer mode#1259
Merged
TruongQuangSB merged 18 commits intomainfrom Jan 21, 2025
Merged
[siteplan] show red yellow comparism also in non-developer mode#1259TruongQuangSB merged 18 commits intomainfrom
TruongQuangSB merged 18 commits intomainfrom
Conversation
Test Results121 tests ±0 121 ✅ ±0 18s ⏱️ -1s Results for commit 493bde2. ± Comparison against base commit ada4872. This pull request removes 9 and adds 9 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
TruongQuangSB
requested changes
Jan 20, 2025
...s/org.eclipse.set.feature.siteplan/src/org/eclipse/set/feature/siteplan/SiteplanBrowser.java
Outdated
Show resolved
Hide resolved
...s/org.eclipse.set.feature.siteplan/src/org/eclipse/set/feature/siteplan/SiteplanBrowser.java
Outdated
Show resolved
Hide resolved
| return [ | ||
| featureClass | ||
| .getFeatures(model.commonState) | ||
| .map(feature => featureClass.setFeatureColor(feature, SiteplanColorValue.COLOR_UNCHANGED_PLANNING)), |
Contributor
There was a problem hiding this comment.
@mariusheine here shouldn't set color to COLOR_UNCHANGED_PLANNING, because when the elemete isn't planing object then it should be grau. See LageplanFeature #getRegionColor()
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB But then it is probably also wrong to set the color to UNCHANGED_PLANNING in initial and final state, isn't it?
Contributor
There was a problem hiding this comment.
@mariusheine yes, in initial and final state shouldn't set color to UNCAHNGED_PLANNING
| const rgbColorString = `rgb(${label.color[0]}, ${label.color[1]}, ${label.color[2]})` | ||
| labelsvg.setAttribute('fill', rgbColorString) | ||
| labelsvg.setAttribute('stroke', rgbColorString) | ||
| } |
Contributor
There was a problem hiding this comment.
@mariusheine i think, we don't need set label color here. We already use svg filter for set svg color (see SvgColorService).
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB Yeah but it somehow was not applied for the signal km marker
Contributor
There was a problem hiding this comment.
@mariusheine the reason is the Label not contain ObjectColor. Because of this can't find the objectColor property in SvgService #getSVGStyle line 94. You can change the propery color in Label to objectColors with type ObjectColor. It will be work
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB This did not worked totally, because the SvgService tried to get the objectColors from the drawData (which was the attachedSignals[0] of the signalMount). So I had to set the correct drawData (the signalMount in this case) and it worked nicely.
Contributor
There was a problem hiding this comment.
@mariusheine the drawData isn't only attachedSignals[0], it is contain attachedSignals[0], featureType, label. But it ist fine, when you change to signalMount, By SingalRouteMarker doesn't matter attachedSignals[0] or signalMount
web/siteplan/src/model/Signal.ts
Outdated
| Mast = 'mast', | ||
| Additive = 'additive', | ||
| RouteMarker = 'marker' | ||
| RouteMarker = 'label' |
Contributor
There was a problem hiding this comment.
@mariusheine here should be still marker. The signal label and marker are different
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB Ok let me try to find a different solution. Without that change it was not easily possible to set the correct color on the marker
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB And the reason why it is not working is because we are statically assigning the id="label" to the signal route marker in SvgDraw#getRouteMarker. Any idea how to fix this?
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB Ok I found a solution for this. See my last commit
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB By thinking about this I came across that I did not set the correct color on the signal label 🙈 Let me check further
Contributor
There was a problem hiding this comment.
@mariusheine u can simple Add enum Label to SignalPart and modify SignalFeature#setFeatureRegionColor to colored the label
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB Nice that i exactly what I test at the moment. Seems to work without any issue
Contributor
Author
There was a problem hiding this comment.
@TruongQuangSB Pushed it. Please review it one last time
| true | ||
| )) | ||
| ) | ||
| labelSvg.setAttribute('id', 'marker') |
This comment has been minimized.
This comment has been minimized.
TruongQuangSB
approved these changes
Jan 21, 2025
License summary❌ Not yet vetted dependencies:
Workflow run (with attached summary files): |
Contributor
|
/license-check |
License summary✔️ All licenses already successfully vetted. Workflow run (with attached summary files): |
TruongQuangSB
pushed a commit
to SuB-PlanPro/set
that referenced
this pull request
Jan 23, 2025
…pse-set#1259) * remove development mode flag * save siteplan and configuration json to files in development mode * overwrite existing object color * explicitly set everything to black for initial and final state * apply color on all parts of a signal * apply label color properly for route markers * also color track switch parts at once * remove obosolete comment * remove unnecessary code * revert static coloring of initial, final and common state * properly coloring signal route marker * properly coloring signal * revert changes in track switch feature * revert unnecessary change * keep marker as signal part name * apply color also on signal labels * use enum instead of string
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.