Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

Bug/worker inspector indentation #1480

Merged
merged 11 commits into from
Sep 14, 2020
Merged

Conversation

BryanJY-Wong
Copy link
Contributor

@BryanJY-Wong BryanJY-Wong commented Sep 9, 2020

Description

Fixed the indentation issue in the Worker Inspector due to nesting of VisualElements

Tests

Created dummy components with various nesting levels of collections and check the indentation in the Worker Inspector manually
null-op
nonnull-op
nested-collections
enum-list
map-indent

Documentation

How is this documented (for example: release note, upgrade guide, feature page, in-code documentation)?

  • Changelog

Primary reviewers

If your change will take a long time to review, you can name at most two primary reviewers who are ultimately responsible for reviewing this request. @ mention them.

@improbable-prow-robot improbable-prow-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. jira/no-ticket Indicates a PR has no corresponding JIRA ticket labels Sep 9, 2020
@improbable-prow-robot improbable-prow-robot added the size/M Denotes a PR that changes 40-149 lines, ignoring generated files. label Sep 9, 2020
@BryanJY-Wong BryanJY-Wong marked this pull request as ready for review September 9, 2020 14:49
@improbable-prow-robot improbable-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 9, 2020
Comment on lines 142 to 145
var varDef = newVariable ? "var " : "";
var nestParam = containedType.Category == ValueType.Type ? $", {nestVar} + 1" : "";

if (newVariable)
{
yield return $"var {uiElementName} = new {inner}(\"{label}\");";
}
else
{
yield return $"{uiElementName} = new {inner}(\"{label}\");";
}
yield return $"{varDef}{uiElementName} = new {inner}(\"{label}\"{nestParam});";
Copy link
Contributor

Choose a reason for hiding this comment

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

This is difficult to understand what's going on here. Can we refactor this function to make this clearer?

Comment on lines 5 to 13
public static class VisualElementExtensions
{
public static void ShiftRightMargin(this VisualElement element, uint nest, float offset = -11)
{
var style = element.style;
var val = style.marginRight.value.value;
style.marginRight = new StyleLength(val + offset * nest);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd possibly make this internal and expose the assembly internals to the generated code.

Probably wouldn't want this to clutter code completion for users using VisualElements elsewhere.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@BryanJY-Wong BryanJY-Wong merged commit f9fa4da into develop Sep 14, 2020
@improbable-prow-robot improbable-prow-robot deleted the bug/worker-inspector-indentation branch September 14, 2020 09:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
jira/no-ticket Indicates a PR has no corresponding JIRA ticket size/M Denotes a PR that changes 40-149 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants