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

Nyxt-native dialogs! #1465

Merged
merged 5 commits into from
Jun 18, 2021
Merged

Nyxt-native dialogs! #1465

merged 5 commits into from
Jun 18, 2021

Conversation

aartaka
Copy link
Contributor

@aartaka aartaka commented May 30, 2021

This adds handing of script-dialog WebKit signal and opens our pretty prompt-buffer instead of WebKit menus, and echoes JS alerts to the message-buffer instead of creating small alerts getting in the way.

@aartaka
Copy link
Contributor Author

aartaka commented May 30, 2021

A small screenshot to get you teased :)
script-dialog2

@aartaka
Copy link
Contributor Author

aartaka commented May 30, 2021

Keep in mind that it only works on the latest version of cl-webkit.

@aartaka aartaka force-pushed the native-script-dialog branch from f41ed94 to b895317 Compare June 2, 2021 14:35
@aartaka
Copy link
Contributor Author

aartaka commented Jun 2, 2021

Mergeable. Thing to do:

  • Before merging this:
    • Update submodule version for cl-webkit.
  • After merging:
    • Add mouse support to prompt-buffer, because we need support equal amount of input devices as built-in dialogs (#1213).
    • Kill web-views properly, so that user gets "Are you sure you want to close this page and lose your input there?" messages.

@aartaka
Copy link
Contributor Author

aartaka commented Jun 7, 2021

I've added file-chooser handing with Nyxt-native interface. It handles multiple file input elegantly, fetches an already existing value and works nice in general. I believe something can be done to color-chooser too, but for that we need to somehow add color parsing (is there any CSS-compatible library for that?).

In general, lots of WebKit UI can be replaced with prompt-buffer (including permission requests and notifications). Maybe introduce a browser-based switch for Nyxt-native UIs? Something like nyxt-native-dialogs.

@jmercouris
Copy link
Member

I think a browser based switch is a good idea! I can't think of a color picker outside of the one when you invoke open-inspector. There are many JS based ones of course... Maybe we could provide a prompt buffer that includes a few sources for different 'standard' colors?

@jmercouris
Copy link
Member

Nothing is to stop us for providing an emacs-like color picker experience :-D

@aartaka
Copy link
Contributor Author

aartaka commented Jun 7, 2021

I think a browser based switch is a good idea! I can't think of a color picker outside of the one when you invoke open-inspector. There are many JS based ones of course...

What we can do is compile user input to CSS, insert it to a page and fetch its color with JavaScript. Terribly hacky, but really convenient. We can even use prompt-buffer's input line as this styled element!

Maybe we could provide a prompt buffer that includes a few sources for different 'standard' colors?

Yes, that could be an option too.

@jmercouris
Copy link
Member

I don't think most people are comfortable translating from hex to a color. I can imagine what a color would be, but I can't be certain, and I've been doing it a long time. I guess it could be the first 'raw' user input source in the prompt buffer.

@aartaka
Copy link
Contributor Author

aartaka commented Jun 7, 2021

I don't think most people are comfortable translating from hex to a color. I can imagine what a color would be, but I can't be certain, and I've been doing it a long time. I guess it could be the first 'raw' user input source in the prompt buffer.

I mean, CSS supports lots of color names. You don't need to necessarily use #HEX or rgb(35, 100, 20) to set the color. That's why CSS is a good way to go here :)

@jmercouris
Copy link
Member

That works too :-)

@jmercouris jmercouris force-pushed the native-script-dialog branch from e59243f to 76e91e5 Compare June 10, 2021 09:35
@jmercouris
Copy link
Member

Hello Artyom, I am taking a look at this now :-)

@aartaka
Copy link
Contributor Author

aartaka commented Jun 10, 2021

Be aware that there's no toggle to turn these Nyxt dialogs on/off yet!

@jmercouris
Copy link
Member

Hm, then I will not merge for now.

@aartaka
Copy link
Contributor Author

aartaka commented Jun 10, 2021

And I also have a color-chooser somewhere in stashes :)

@Ambrevar
Copy link
Member

Ambrevar commented Jun 10, 2021 via email

@aartaka aartaka force-pushed the native-script-dialog branch from 76e91e5 to 99ef351 Compare June 10, 2021 14:46
@aartaka
Copy link
Contributor Author

aartaka commented Jun 10, 2021

OK, here's the native-dialogs switch! We can merge if it doesn't break on CI :)

Color-chooser proved to be quite complex, because getting the color of DOM element is a non-obvious task, while I need it to leverage all the power of CSS. Let's leave it for later and open an issue on it.

@aartaka
Copy link
Contributor Author

aartaka commented Jun 10, 2021

It doesn't seem to break on CI :D

@Ambrevar
Copy link
Member

Ambrevar commented Jun 15, 2021 via email

source/renderer-gtk.lisp Outdated Show resolved Hide resolved
@aartaka
Copy link
Contributor Author

aartaka commented Jun 15, 2021

All adressed! (I resorted to a suspicious ignore-errors to return nil from the cancelled prompt, but it's all smooth otherwise.)

@aartaka
Copy link
Contributor Author

aartaka commented Jun 17, 2021

The last suspicious thing (the ignore-erros) is addressed in 25dbdc5. Should be mergeable, I believe.

@jmercouris
Copy link
Member

Thanks Artyom!

@Ambrevar Ambrevar merged commit a583172 into master Jun 18, 2021
@jmercouris jmercouris deleted the native-script-dialog branch July 28, 2021 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants