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.

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

Remove ViewModel wording from debug guide #5425

Merged
merged 4 commits into from Nov 19, 2019

Conversation

@cherifGsoul
Copy link
Member

cherifGsoul commented Nov 18, 2019

  • Update the debugging guide by replacing ViewModel words by Observable Properties
  • Update devtools screenshots by new ones that contain Observable Properties

For canjs/devtools#78

@cherifGsoul cherifGsoul requested a review from phillipskevin Nov 18, 2019
Copy link
Collaborator

phillipskevin left a comment

Added a few comments... the biggest thing is observable properties should be lowercase if it is not part of a proper noun like Observable Properties Editor.

docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
docs/can-guides/topics/debugging.md Outdated Show resolved Hide resolved
@cherifGsoul

This comment has been minimized.

Copy link
Member Author

cherifGsoul commented Nov 18, 2019

@phillipskevin I updated the wording to lowercase , thanks!

@cherifGsoul cherifGsoul requested a review from phillipskevin Nov 18, 2019
Copy link
Contributor

bmomberger-bitovi left a comment

Some other docs where we should change the terminology:

https://github.com/canjs/canjs/blame/obserbale-properties-update-debugging-guid/docs/can-guides/introduction/technical.md#L716 (and other references to view models in this doc)

https://github.com/canjs/canjs/blame/obserbale-properties-update-debugging-guid/docs/can-guides/experiment/technology/technology.md#L211

https://github.com/canjs/canjs/blame/obserbale-properties-update-debugging-guid/docs/can-guides/experiment/atm/atm.md#L104

There's also several references to view model in the can6 upgrade guide docs/can-guides/upgrade/migrating-to-canjs-5.md , but your call whether you want to change those.

Copy link
Collaborator

phillipskevin left a comment

One small change then go ahead and merge. We can open separate issues for the things @ bmomberger-bitovi pointed out.

@@ -571,14 +571,14 @@ Then, depending on the size of the Developer Tools window, the CanJS Bindings Gr
alt="The Chrome Developer Tools Sidebar Overflow Menu"
width="600px"/>

If the element that is selected has a Observable Properties, the graph will show the relationships for a property of the Observable Properties:
If the element that is selected has a observable properties, the graph will show the relationships for a property:

This comment has been minimized.

Copy link
@phillipskevin

phillipskevin Nov 18, 2019

Collaborator

has observable properties

cherifGsoul added 2 commits Nov 19, 2019
@cherifGsoul cherifGsoul merged commit 66d7d7f into master Nov 19, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@cherifGsoul cherifGsoul deleted the obserbale-properties-update-debugging-guid branch Nov 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.