Skip to content

Conversation

reubenmiller
Copy link
Contributor

@reubenmiller reubenmiller commented May 21, 2025

Proposed changes

Update screenshots to reflect the current Cumulocity branding.

  • Use consistent window sizing / aspect ratio (where possible)
  • Remove browser information
  • Removed some screenshots which were deemed as unnecessary as they didn't provide any additional value

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
  • Documentation Update (if none of the other choices apply)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue


Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check as mentioned in CODING_GUIDELINES
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

Further comments

@reubenmiller reubenmiller added the documentation Improvements or additions to documentation label May 21, 2025
Copy link

codecov bot commented May 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@didier-wenzek didier-wenzek left a 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>
@reubenmiller reubenmiller force-pushed the docs-update-screenshots branch from 204f785 to 6e20ae3 Compare May 22, 2025 14:33
@reubenmiller reubenmiller force-pushed the docs-update-screenshots branch from 6e20ae3 to 948abd3 Compare May 22, 2025 21:56
Signed-off-by: reubenmiller <reuben.d.miller@gmail.com>
@reubenmiller reubenmiller force-pushed the docs-update-screenshots branch from 948abd3 to 6a31926 Compare May 22, 2025 22:15
@reubenmiller reubenmiller temporarily deployed to Test Pull Request May 22, 2025 22:15 — with GitHub Actions Inactive
@reubenmiller reubenmiller marked this pull request as ready for review May 22, 2025 22:15
Copy link
Contributor

@didier-wenzek didier-wenzek left a 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* &rarr; *Device profiles* page

<BrowserWindow url="https://example.cumulocity.com/apps/devicemanagement/index.html#/profiles">

![Create new device profile](images/device-profile-repo-1-list.png)
Copy link
Contributor

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 (![image](..)) while in other place an HTML <img> is used?

Copy link
Contributor Author

@reubenmiller reubenmiller May 23, 2025

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.

Comment on lines +377 to +379
<p align="center">
<img width="40%" src={require('./images/AddSoftware.png').default} alt="Add new software" />
</p>
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
634 0 3 634 100 1h49m25.237156999s

@reubenmiller reubenmiller added this pull request to the merge queue May 23, 2025
Merged via the queue into thin-edge:main with commit 27ea223 May 23, 2025
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants