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

UI - Allow editors in full screen mode #1805

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

thfries
Copy link
Contributor

@thfries thfries commented Nov 12, 2023

Hello @thjaeckle,

I reworked the editor size behavior:

  • all editors allow a new fullscreen attribute
  • while editor is active, you can toggle the mode. The mode per editor is remembered by the component
  • fixed also the scrolling behavior in full screen mode. Full screen is full screen - no scrolling.

Please let me know if that works for you.

@thjaeckle thjaeckle added the UI Issues related to the Ditto explorer UI label Nov 13, 2023
@thjaeckle thjaeckle added this to the 3.5.0 milestone Nov 13, 2023
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

Hi @thfries
For me, the "full screen" button (for Thing editor, but also Feature editor) apparently does not work. I am using Chrome and when I click it, I no longer see an editor at all, only the "greyed out" background.
With which browser did you test it?

In the "Console" I see:

features.ts:247 Uncaught (in promise) TypeError: Cannot read properties of null (reading 'replace')
    at features.ts:247:45
(anonymous) @ features.ts:247
Promise.then (async)
onEditToggle4 @ features.ts:250
toggleEdit @ crudToolbar.ts:161
dom.buttonEdit.onclick @ crudToolbar.ts:111

@thfries
Copy link
Contributor Author

thfries commented Nov 21, 2023

Hi @thjaeckle, that is strange. I am also testing with Chrome. Could that be caused by old cached css classes? Can you try shift-reload in Chrome? If that is the reason, we need some way to force the Browser to reload somehow...

The error on the console is the same as on the policy-Imports PR caused by things without features. Fix will be merged there.

Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

Aaah, it works now 👍

I think I did not restart npm run start when switching reviewing from the other UI PR.
I did that now and it works.

So looks good to me, also a nice addition 👍

@thjaeckle thjaeckle merged commit 2d6d55b into eclipse-ditto:master Nov 21, 2023
3 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Issues related to the Ditto explorer UI
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants