-
Notifications
You must be signed in to change notification settings - Fork 66
docs: update screenshots #3637
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
docs: update screenshots #3637
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
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.
A bit difficult to test before thin-edge/tedge-docs#114 (review) being merged.
- Merged meantime.
- This new
BrowserWindow
MDX component is really neat, notably with this url bar.
The updated snapshots look good.
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
204f785
to
6e20ae3
Compare
6e20ae3
to
948abd3
Compare
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
948abd3
to
6a31926
Compare
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.
Approved. A thorough and very nice looking improvement
@@ -18,25 +20,33 @@ A device profile can be created by using the following steps. Afterwards, the de | |||
|
|||
1. Navigate to the *Management* → *Device profiles* page | |||
|
|||
<BrowserWindow url="https://example.cumulocity.com/apps/devicemanagement/index.html#/profiles"> | |||
|
|||
 |
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.
Question. Is there a logic to prefer in some case a markdown (
) while in other place an HTML <img>
is used?
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 main reason is to allow controlling the size of the image (<img>
supports scaling the size), e.g. if it needs to be scaled down. But I'm sure there is some inconsistencies here and there.
<p align="center"> | ||
<img width="40%" src={require('./images/AddSoftware.png').default} alt="Add new software" /> | ||
</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.
The nice frame of a BrowserWindow
is missing here, as image and text backgrounds are both white.
Can this frame captured with the image? As done for docs/src/start/images/add-new-software-to-repo-remote.png
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.
I mostly added the BrowserWindow for full screenshots, where as this is just a screenshot of the dialog box...so the BrowserWindow looked a bit weird...but maybe we should just use it anyway.
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.
That's okay. Indeed, the BrowserWindow looked a bit weird when wrapping a dialog box which is then lost in middle of an empty screen.
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.
Ok, so we'll leave it has is (don't use BrowserWindow for screenshots of partial screens). In the future we might spend some time to come up with alternative, or some css to make it look nicer.
Robot Results
|
Proposed changes
Update screenshots to reflect the current Cumulocity branding.
Types of changes
Paste Link to the issue
Checklist
just prepare-dev
once)just format
as mentioned in CODING_GUIDELINESjust check
as mentioned in CODING_GUIDELINESFurther comments