More designer fixes #3169

dclaux · 2019-07-10T21:41:11Z

Major designer update, plus fixes for:

Verified manually.

Microsoft Reviewers: Open in CodeFlow

- adaptivecards-controls@0.3.0 - adaptivecards-data@0.1.0-alpha1 - adaptivecards-designer-app@0.2.1 - adaptivecards-designer@0.7.0 - adaptivecards-site@0.4.1 - hexo-theme-adaptivecards@0.1.1 - adaptivecards-visualizer@1.2.1 - adaptivecards@1.3.0-alpha1

Opportunistic small bug fixes

[Designer] Basic property categorization in property sheet

- adaptivecards-controls@0.3.0 - adaptivecards-designer-app@0.2.1 - adaptivecards-designer@0.7.0 - adaptivecards-site@0.4.1 - hexo-theme-adaptivecards@0.1.2 - adaptivecards-templating@0.1.0-alpha.0 - adaptivecards-visualizer@1.2.2 - adaptivecards@1.3.0-alpha1 - spec-generator@0.5.1 - typed-schema@0.5.1

shalinijoshi19

Could you please create specific topic branches for the changes here - One is sufficient but this is ulling in reviews and commits from a now committed set of changes which is confusing to review. Thanks. Please reach out to @paulcam206 for help.

paulcam206

per meeting review, this is approved pending the one outstanding issue found.

dclaux · 2019-08-28T20:43:53Z

source/nodejs/adaptivecards/src/utils.ts

-    } else {
+    }
+    else if (typeof value === "number") {
+        return value ? value : defaultValue;


Shouldn't be necessary? Value is a number at this point and therefore can be returned (0 is meaningful)

shalinijoshi19 · 2019-08-28T21:11:22Z

Hi David, In general do you have a list of known cleanup/refactoring in the codebase that you are working on or planning to work on at all? If there is could you let me know the issue# (all-up issue fine) for that? Also some of these might be able to be found and potentially fixed with a linter (es-lint) so we can potentially resolve any existing issues once we enable a linting tool for the codebase. Thanks!

shalinijoshi19 · 2019-08-28T21:12:29Z

Are you able to update the description to be more descriptive? Thanks

shalinijoshi19 · 2019-08-28T21:14:59Z

For fixes syntax, i believe you have to prefix each issue fixed with a keyword (eg fixes #<>, fixes #<> or else the issues themselves dont get auto-closed. Please refer to https://help.github.com/en/articles/closing-issues-using-keywords#closing-multiple-issues

dclaux · 2019-08-28T21:18:34Z

There isn't such a list. There is an issue open to replace "var" with "let" pretty much everywhere, which I do opportunistically (no real reason to address it all at once.)

There will be one (or two) big refactors needed to enable additional features going forward:

One to allow a fully functional implementation of the version selector in the designer (an issue has already been raised for that feature: [Designer] Version Restriction #3260)
One to cleanup the parsing path in the renderer, which will likely be necessary for logging once we all agree on what needs to be logged

I do not know when I'll be able to do these though. The first one is a large amount of work and is risky.

shalinijoshi19 · 2019-08-28T21:19:57Z

There isn't such a list. There is an issue open to replace "var" with "let" pretty much everywhere, which I do opportunistically (no real reason to address it all at once.)

There will be one (or two) big refactors needed to enable additional features going forward:

One to allow a fully functional implementation of the version selector in the designer (an issue has already been raised for that feature: [Designer] Version Restriction #3260)

One to cleanup the parsing path in the renderer, which will likely be necessary for logging once we all agree on what needs to be logged

I do not know when I'll be able to do these though. The first one is a large amount of work and is risky.

Ok thanks. This is good to know and helpful context. Hooking up a linter should help clean up low hanging fruit like var usage I am hoping.

dclaux · 2019-08-28T21:20:18Z

The description is accurate, I list the issues that this PR fixes. Is there something else you'd like in there? I could describe what I actually did, but that'd be redundant with the code changes themselves.

shalinijoshi19 · 2019-08-28T21:25:52Z

The description is accurate, I list the issues that this PR fixes. Is there something else you'd like in there? I could describe what I actually did, but that'd be redundant with the code changes themselves.

Yes please.

Fixing up the usage of the Fixes keyword to enable the github auto-closure logic to run
Is there a way for each that you can summarize what was broken ? The code changes would typically reflect how you fixed it, but not necessarily what was broken, which is where the description helps. If you noticed the template, there was a link that went at length if you wanted to read up but that;s more for reference really _(it helps with this and potential reverts should they be needed at any point, if we can avoid bundling in too many disparate changes in one PR/commit) _
How Verified looks OK to me.

shalinijoshi19 · 2019-08-28T21:27:56Z

Btw, you may want to update from master to pull in the latest and greatest and verify things still look good (looks like your branch is out of date with master).

dclaux · 2019-08-28T21:44:35Z

I have updated to uses the proper "fixes" syntax
The issues already describe what was broken, what else would you like to see in this PR?

dclaux · 2019-08-28T22:22:34Z

All my changes are unrelated to the UWP renderer and do not touch its codebase. I am ignoring the failure for that reason.

dclaux and others added 30 commits May 29, 2019 14:36

Inject TS source from js/template-engine

5db2c2a

Post build changes

ddb2739

Cleanup

47531f1

Fix compilation issues (WIP)

05e85dd

Merge branch 'master' into clean-template-engine-branch

822b1ad

Merge branch 'master' into clean-template-engine-branch

0f0135d

Fixes packaging problems

325dfaf

Merge branch 'master' into clean-template-engine-branch

39159de

Update README

eee0ed7

Publish

a75f999

- adaptivecards-controls@0.3.0 - adaptivecards-data@0.1.0-alpha1 - adaptivecards-designer-app@0.2.1 - adaptivecards-designer@0.7.0 - adaptivecards-site@0.4.1 - hexo-theme-adaptivecards@0.1.1 - adaptivecards-visualizer@1.2.1 - adaptivecards@1.3.0-alpha1

adaptivecards-data --> adaptivecards-templating

e7c52c6

Update README for adaptivecards-templating

b308d72

Fix a couple typos in README

4c92da9

Merge branch 'master' into clean-template-engine-branch

28a6938

Significant designer code simplification (WIP)

229aec8

Opportunistic small bug fixes

More designer code simplification

26da92c

Finalize designer code simplification

81bb71d

Merge branch 'master' into clean-template-engine-branch

b63a4ad

Fix minHeight serialization regression

f78d1ab

[Designer] Basic versioning support

7e533de

[Designer] Basic property categorization in property sheet

Merge branch 'master' into clean-template-engine-branch

6f83770

[JS] Hotfix bleed regression in ColumnSet

1193c22

cleaning up sample payloads, fixing some site bugs

dd80654

[Designer] Hide more preview features

eab64e7

Merge branch 'master' into clean-template-engine-branch

5563e49

Update package-lock.json

2355ee1

dts not working, revert tnhis commit

ce49897

Merge branch 'master' into clean-template-engine-branch

7d315ce

Merge branch 'master' into clean-template-engine-branch

cd454de

shalinijoshi19 suggested changes Aug 27, 2019

View reviewed changes

ghost added the Needs: Author Feedback label Aug 27, 2019

Merge branch 'master' into clean-template-engine-branch

b13cdf4

ghost removed the Needs: Author Feedback label Aug 27, 2019

dclaux mentioned this pull request Aug 27, 2019

[Designer][Designer adds property that's not part of schema for Inputs] #3402

Closed

jwoo-msft mentioned this pull request Aug 27, 2019

[UWP] Visulizer throwing an futile error expecting number on strings #3178

Closed

Merge branch 'master' into clean-template-engine-branch

fde283b

paulcam206 approved these changes Aug 28, 2019

View reviewed changes

Address code review comments

6404f81

dclaux commented Aug 28, 2019

View reviewed changes

shalinijoshi19 approved these changes Aug 28, 2019

View reviewed changes

Merge branch 'master' into clean-template-engine-branch

b35bc51

dclaux merged commit 1f534d6 into master Aug 28, 2019

dclaux deleted the clean-template-engine-branch branch August 28, 2019 22:22

jodufra mentioned this pull request Sep 6, 2019

[Question] What is the estimated release date of v1.2.2 in npm? #3422

Closed

shalinijoshi19 added AdaptiveCards v1.2.3 Bug and removed Bug labels Sep 13, 2019

More designer fixes #3169

More designer fixes #3169

Uh oh!

Conversation

dclaux commented Jul 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Microsoft Reviewers: Open in CodeFlow

Uh oh!

shalinijoshi19 left a comment

Choose a reason for hiding this comment

Uh oh!

paulcam206 left a comment

Choose a reason for hiding this comment

Uh oh!

dclaux Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

dclaux Aug 28, 2019

Choose a reason for hiding this comment

Uh oh!

shalinijoshi19 commented Aug 28, 2019

Uh oh!

shalinijoshi19 commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shalinijoshi19 commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dclaux commented Aug 28, 2019

Uh oh!

shalinijoshi19 commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dclaux commented Aug 28, 2019

Uh oh!

shalinijoshi19 commented Aug 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shalinijoshi19 commented Aug 28, 2019

Uh oh!

dclaux commented Aug 28, 2019

Uh oh!

dclaux commented Aug 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dclaux commented Jul 10, 2019 •

edited

Loading

shalinijoshi19 commented Aug 28, 2019 •

edited

Loading

shalinijoshi19 commented Aug 28, 2019 •

edited

Loading

shalinijoshi19 commented Aug 28, 2019 •

edited

Loading

shalinijoshi19 commented Aug 28, 2019 •

edited

Loading