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

payg: Create PaygAddCreditDialog #745

Draft
wants to merge 230 commits into
base: master
Choose a base branch
from
Draft

payg: Create PaygAddCreditDialog #745

wants to merge 230 commits into from

Conversation

jprvita
Copy link
Contributor

@jprvita jprvita commented Apr 7, 2023

PaygAddCreditDialog is a modal dialog for early code entry, to replace
the notification widget currently in use.

https://phabricator.endlessm.com/T31748

SelyanKab and others added 30 commits October 16, 2021 14:35
If we get '' from the IM as the preedit string, it is pretty safe
to handle it as "no preedit" altogether.

(Cherry-picked from commit 286cfdc)
This is now done in the ClutterInputFocus for us, since
https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1940. There
is no need to duplicate this call, and it happens to cause undesired
scrolling to content in some cases when this reset happens during
a change in the IM focused client state.

Fixes: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4647

(Cherry-picked from commit 047691b)
If the preedit is in invisible state, the last preedit string that
was sent around is already null, so there is no need to clear the preedit
in that case.

(Cherry-picked from commit 0177560)
Like the old Tweener API, ease() allows to transition multiple
properties at once. If autoReverse or repeatCount are specified,
they should apply to all transitions, but right now we only set
them for the first one.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2007>
(cherry picked from commit cc5cc0d)
…PI exists"

Since we are past the 40 API change hurdle, it is safe to revert this
commit.

This reverts commit 1cefd58.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2010>
(cherry picked from commit ff4ff66)
Windows from some applications, such as guake, are created as showing on
all workspaces. When these windows are put on the workspaces via
set_workspace_state() during construction, the first time the window is
added to a workspace in the loop triggers the shell app tracker which
then tries to move the window to its startup workspace. This makes the
window unsticky which triggers another set_workspace_state() which
tries to remove the window from all workspaces, but currently it is only
on the first one and then adds it to the startup workspace. Once that is
finished, the first set_workspace_state() continues adding the window
to the remaining workspaces, despite the window now no longer having
on_all_workspaces set to true.

When the window is now unmanaged, the window according to its internal
state is only found on the startup workspace, so it will only be removed
from that. This causes the assertion to fail that checks that the window
is no longer present on any workspace after this.

Fixes https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4720

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2014>
(cherry picked from commit bc32a52)
Unlike g_warning() and friends, g_printerr() does not append
a trailing newline.

https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4564

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2013>
(cherry picked from commit 79f4489)
We currently ignore any error that may occur when calling the
OpenExtensionPrefs D-Bus method. Right now such an error is highly
unlikely, given that we already checked that we are running under
gnome-shell and the extension in question exists and has prefs.

We'll soon make sure that only one dialog is shown at any time,
which is an error that we can realistically expect, so handle that
properly.

https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4564

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2013>
(cherry picked from commit 09ed1c5)
GTK4 relies entirely on refcounting for cleanup (that is,
there is no longer a destroy() method that forces a dispose
run regardless of the refcount).

Unfortunately that makes cleanup harder in (some) language
bindings, where an object may be kept alive implicitly by
closures etc.

Address this by releasing the hold count when the window
is closed rather than when it is destroyed.

This isn't the most elegant, but it ensure that the service
doesn't get stuck if an extension doesn't carefully clean
up everything in its prefs widget.

https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4564

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2013>
(cherry picked from commit 4d2b008)
Showing multiple preference dialogs at the same time (for instance
by repeated `gnome-extensions prefs` calls) may or may not work as
expected, depending on whether any of the dialogs is modal or not
(read: opened via the Extensions app).

The easiest way to address this is to disallow more than a single
dialog at the time. It's arguably also the more predictable behavior,
and means extensions don't have to deal with inconsistent state
caused by multiple dialogs.

https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4564

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2013>
(cherry picked from commit b93342f)
The change unvealed a series of mutter bugs that for most
users are worse than the issue that was fixed. So revert
the change on stable branches until the mutter side is
fixed.

This reverts commit cd1102f.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2022>
The changes introduce crashes, so back out until we
figure those out.

This reverts commits ceee53a and 5d5bfe4.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2022>
Jenkins and others added 6 commits March 28, 2023 19:06
This commit brings no functional change.
This commit brings no funcional changes.
PaygAddCreditDialog is a modal dialog for early code entry, to replace
the notification widget currently in use. This also creates a
PaygAddCreditDialog object from status/payg and adds a separate action
to open it.

https://phabricator.endlessm.com/T31748
This drops the ApplyCodeNotification class and the status menu entry
that triggers it. It is being replaced by PaygAddCreditDialog.

https://phabricator.endlessm.com/T31748
js/ui/payg.js Outdated Show resolved Hide resolved
js/ui/payg.js Outdated Show resolved Hide resolved
js/ui/payg.js Outdated Show resolved Hide resolved
js/ui/payg.js Outdated Show resolved Hide resolved
js/ui/payg.js Show resolved Hide resolved
js/ui/payg.js Show resolved Hide resolved
js/ui/payg.js Outdated Show resolved Hide resolved
js/ui/payg.js Outdated Show resolved Hide resolved
js/ui/payg.js Outdated Show resolved Hide resolved
@jprvita
Copy link
Contributor Author

jprvita commented Apr 13, 2023

I've addressed all comments up to this point, and fixed two small bugs:

  • Closing the dialog after entering a reset code, so it does not remain over the lock shield;
  • Resetting _verificationStatus in _reset() so the sensitivities are updated correctly (honestly I believe we could vastly simplify _verificationStatus, but this was a lot of c&p from ApplyCodeNotification, PaygUnlockWidget and PaygUnlockUi, so I rather keep it as-is for now to reduce the potential for introducing bugs.

@jprvita jprvita marked this pull request as draft April 13, 2023 19:40
@jprvita
Copy link
Contributor Author

jprvita commented Apr 13, 2023

Converted to draft so we don't accidentally merge with all the fixup commits -- I'll squash them later. Also, I'll experiment with Cassidy's suggestion in the ticket, and ask some feedback from Joana before this is ready for merging.

No need to mention the inheritance in a comment, as it is pretty clear
in the class declaration.
@jprvita
Copy link
Contributor Author

jprvita commented May 1, 2023

Added my non-working attempt at using a label that expands for the results messages.

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.