Refactor/2318/migrate attribute type selector #2519
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| import { createSelector } from "../angular-redux/store" | ||
| import { attributeTypesSelector } from "../store/fileSettings/attributeTypes/attributeTypes.selector" | ||
| import { CcState } from "../store/store" | ||
| import { AttributeTypeValue } from "../../codeCharta.model" | ||
|
|
||
| export type GetAttributeTypeOfNodesByMetric = (metricName: string) => AttributeTypeValue | ||
|
|
||
| export const getAttributeTypeOfNodesByMetricSelector: (state: CcState) => GetAttributeTypeOfNodesByMetric = createSelector( | ||
| [attributeTypesSelector], | ||
| attributeTypes => (metricName: string) => attributeTypes.nodes[metricName] | ||
| ) |
This file contains 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
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { AttributeTypeValue } from "../../codeCharta.model" | ||
| import { AggregationSymbolPipe } from "./aggregationSymbol.pipe" | ||
|
|
||
| describe("aggregationSymbolPipe", () => { | ||
| it("should map `AttributeTypeValue.relative` to x͂", () => { | ||
| const getAttributeTypeOfNodesByMetricSelector = () => AttributeTypeValue.relative | ||
| expect(new AggregationSymbolPipe().transform("some-metric-name", getAttributeTypeOfNodesByMetricSelector)).toBe("x͂") | ||
| }) | ||
|
|
||
| it("should map `AttributeTypeValue.absolute` to Σ", () => { | ||
| const getAttributeTypeOfNodesByMetricSelector = () => AttributeTypeValue.absolute | ||
| expect(new AggregationSymbolPipe().transform("some-metric-name", getAttributeTypeOfNodesByMetricSelector)).toBe("Σ") | ||
| }) | ||
|
|
||
| it("should default to Σ", () => { | ||
| const getAttributeTypeOfNodesByMetricSelector = () => "" as AttributeTypeValue | ||
| expect(new AggregationSymbolPipe().transform("some-metric-name", getAttributeTypeOfNodesByMetricSelector)).toBe("Σ") | ||
| }) | ||
| }) |
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| import { Pipe, PipeTransform } from "@angular/core" | ||
|
|
||
| import { AttributeTypeValue } from "../../codeCharta.model" | ||
| import { GetAttributeTypeOfNodesByMetric } from "../../state/selectors/getAttributeTypeOfNodesByMetric.selector" | ||
|
|
||
| @Pipe({ name: "aggregationSymbolPipe" }) | ||
| export class AggregationSymbolPipe implements PipeTransform { | ||
| transform(metricName: string, getAttributeTypeOfNodesByMetricSelector: GetAttributeTypeOfNodesByMetric): string { | ||
| const type = getAttributeTypeOfNodesByMetricSelector(metricName) | ||
| switch (type) { | ||
| case AttributeTypeValue.relative: | ||
| return "x͂" | ||
| case AttributeTypeValue.absolute: | ||
| default: | ||
| return "Σ" | ||
| } | ||
| } | ||
| } |
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,12 @@ | ||
| <div class="small-action-button transparent"> | ||
| <button mat-button [matMenuTriggerFor]="menu" title="Select attribute type" class="menu-button"> | ||
| <span class="symbol">{{ metric | aggregationSymbolPipe: (getAttributeTypeOfNodesByMetric$ | async) }}</span> | ||
| </button> | ||
| </div> | ||
|
|
||
| <mat-menu #menu> | ||
| <div class="menu-header" mat-menu-item disabled>Set global aggregation for rloc</div> | ||
| <mat-divider></mat-divider> | ||
| <button mat-menu-item (click)="setToAbsolute()">Σ Sum</button> | ||
| <button mat-menu-item (click)="setToRelative()">x͂ Median</button> | ||
| </mat-menu> | ||
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,22 @@ | ||
| cc-attribute-type-selector { | ||
| display: inline-block; | ||
|
|
||
| .menu-button { | ||
| height: 24px; | ||
| width: 24px; | ||
| min-width: 0 !important; | ||
shaman-apprentice marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| background-color: white; | ||
| border-radius: 16px; | ||
| padding: 0; | ||
|
|
||
Christian-Eberhard marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| &:hover { | ||
| background-color: rgb(200, 200, 200); | ||
|
There was a problem hiding this comment. We used #c8c8c8 before, for most hovers. But this is a personal preference to use hex codes over rgb |
||
| } | ||
|
|
||
| .symbol { | ||
| position: absolute; | ||
| top: -6px; | ||
| left: 8px; | ||
| } | ||
| } | ||
| } | ||
This file contains 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,93 +1,53 @@ | ||
| import { TestBed } from "@angular/core/testing" | ||
| import { render, screen, fireEvent, waitForElementToBeRemoved } from "@testing-library/angular" | ||
|
|
||
| import { AttributeTypeSelectorModule } from "./attributeTypeSelector.module" | ||
| import { Store } from "../../state/store/store" | ||
| import { AttributeTypeSelectorComponent } from "./attributeTypeSelector.component" | ||
| import { AttributeTypeValue } from "../../codeCharta.model" | ||
| import { setAttributeTypes } from "../../state/store/fileSettings/attributeTypes/attributeTypes.actions" | ||
|
|
||
| describe("attributeTypeSelector", () => { | ||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({ | ||
| imports: [AttributeTypeSelectorModule] | ||
| }) | ||
|
|
||
| Store["initialize"]() | ||
| Store.store.dispatch( | ||
| setAttributeTypes({ | ||
| nodes: { rloc: AttributeTypeValue.absolute } | ||
| }) | ||
| ) | ||
| }) | ||
|
|
||
| it("should update to median", async () => { | ||
| await render(AttributeTypeSelectorComponent, { | ||
| componentProperties: { metric: "rloc" }, | ||
| excludeComponentDeclaration: true | ||
| }) | ||
|
|
||
| const initialDisplayedElement = await screen.findByText("Σ") | ||
| expect(initialDisplayedElement).toBeTruthy() | ||
|
|
||
| fireEvent.click(initialDisplayedElement) | ||
| const medianMenuItem = await screen.findByText("x͂ Median") | ||
| expect(medianMenuItem).toBeTruthy() | ||
|
|
||
| fireEvent.click(medianMenuItem) | ||
| await waitForElementToBeRemoved(() => { | ||
| const medianMenuItem = screen.queryByText("x͂ Median") | ||
| return medianMenuItem | ||
| }) | ||
| const updatedDisplayedElement = await screen.findByText("x͂") | ||
| expect(updatedDisplayedElement).toBeTruthy() | ||
| }) | ||
|
|
||
| it("should set aggregation symbol to absolute if attributeType is not available", async () => { | ||
| await render(AttributeTypeSelectorComponent, { | ||
| componentProperties: { metric: "non-existing" }, | ||
| excludeComponentDeclaration: true | ||
| }) | ||
| expect(await screen.findByText("Σ")).toBeTruthy() | ||
| }) | ||
| }) |
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal preference, but i think a span or any other generic text element would be cleaner than just writing the text straight into a div, since we could in theory have spans standardized across the project but not generic text in divs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The advantage of this standard isn't clear to me. Anyway, lets not spend time in something which we might do, while "just" migrating to Angular, which is already a very very huge task :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Text just in a div has no semantic meaning for text, its used for layout, while span is used to style texts. If that makes more sense