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

Add response previewer for text/html #243

Closed
wants to merge 6 commits into from

Conversation

monstajoe2002
Copy link

PR Description

Add your description

Related Issues

Checklist

  • I have gone through the contributing guide
  • I have run the tests (flutter test) and all tests are passing

Added/updated tests?

We encourage you to add relevant test cases.

  • Yes
  • No, and this is why: please replace this line with details on why tests have not been included
  • I need help with writing tests

@monstajoe2002 monstajoe2002 mentioned this pull request Mar 2, 2024
@BrawlerXull
Copy link
Contributor

BrawlerXull commented Mar 2, 2024

Hey 👋 the package used in the pr

Have Linux support listed as work in progress at pub dev

I suggest using desktop_webview_window as it supports all the 3 platforms - MacOs , Windows and Linux

@mmjsmohit
Copy link
Contributor

mmjsmohit commented Mar 2, 2024

@BrawlerXull The package desktop_webview_window has some pre-requisites which are necessary to be installed on the user's machine. We would have to figure out how to install the pre-reqs while API Dash is being installed.

See https://pub.dev/packages/desktop_webview_window#linux-requirement and https://pub.dev/packages/desktop_webview_window#windows

@monstajoe2002
Copy link
Author

@BrawlerXull The package desktop_webview_window has some pre-requisites which are necessary to be installed on the user's machine. We would have to figure out how to install the pre-reqs while API Dash is being installed.

See https://pub.dev/packages/desktop_webview_window#linux-requirement

Exactly, this approach also spawns a new webview window, which is not the desired outcome as per the screenshot in the original PR.

@ashitaprasad ashitaprasad changed the title Add feature 165 Add response previewer for text/html Mar 3, 2024
@monstajoe2002
Copy link
Author

Shall I close this request or is someone gonna merge it?

@animator
Copy link
Member

animator commented Mar 3, 2024

@monstajoe2002 It takes time to review and merge PRs. Please be patient.

@ashitaprasad ashitaprasad linked an issue Mar 5, 2024 that may be closed by this pull request
@mmjsmohit
Copy link
Contributor

Hi @monstajoe2002!
I am getting the following error when running your code:

 [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: 'package:webview_cef/src/webview.dart': Failed assertion: line 197 pos 12: 'value': is not true.
#0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:51:61)
#1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:40:5)
#2      WebViewController._cursorMove (package:webview_cef/src/webview.dart:197:12)
#3      WebViewState._buildInner.<anonymous closure> (package:webview_cef/src/webview.dart:310:25)
#4      RenderPointerListener.handleEvent (package:flutter/src/rendering/proxy_box.dart:3084:30)
#5      GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:475:22)
#6      RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:430:11)
#7      GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:420:7)
#8      GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:383:5)
#9      GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:330:7)
#10     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:299:9)
#11     _invoke1 (dart:ui/hooks.dart:328:13)
#12     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:429:7)
#13     _dispatchPointerDataPacket (dart:ui/hooks.dart:262:31)

I am running API Dash on Ubuntu 23.10.

@monstajoe2002
Copy link
Author

I found webview_universal, which should supposedly work on Linux. However, it creates a separate window for that HTML page. Is that OK for you?

@animator
Copy link
Member

animator commented Mar 8, 2024

@monstajoe2002 Please add preview (screenshot) for the test case given here

@monstajoe2002
Copy link
Author

image
Here's the result after using webview_universal

@animator
Copy link
Member

animator commented Mar 8, 2024

@monstajoe2002 Ahh it opens in a new window. This won't work as it will lead to poor user experience.

@animator
Copy link
Member

animator commented Mar 8, 2024

@monstajoe2002 If a solution works for macOS & Windows and does not work for Linux, it is acceptable. We can raise an improvement issue to add support for Linux for that feature so that the community can work on it and add it when it is available.

@monstajoe2002
Copy link
Author

monstajoe2002 commented Mar 8, 2024

Exactly. Do note that I had to modify main.cpp (Desktop binary) in the windows directory on Windows to get it working according to the webview_cef documentation. This is ignored by git by default.

@animator
Copy link
Member

animator commented Mar 8, 2024

One issue with webview_cef is that the build size will increase drastically due to chromium binary. Is there any other alternative?

@monstajoe2002
Copy link
Author

So far, no. How drastic are we talking?

@monstajoe2002
Copy link
Author

Why did you close #165?

@animator
Copy link
Member

So far, no. How drastic are we talking?

Adding ~200 MB just to add this feature is an overkill. The current size is ~30 MB.

Why did you close #165?

As the package used did not provide the desired result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add response previewer for text/html
4 participants