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.

Sign up for GitHub

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

Jump to bottom

feat(StatusQ): create a standard StatusMessageDialog component #17587

Merged
merged 2 commits into from
Mar 19, 2025
Merged

feat(StatusQ): create a standard StatusMessageDialog component #17587

merged 2 commits into from
Mar 19, 2025

Conversation

caybro
Copy link
Member

@caybro caybro commented Mar 13, 2025 •
edited
Loading

What does the PR do

  • (almost) a dropin replacement for the QtQuick.1 MessageDialog, preserving the same API
  • based on StatusDialog to get the default look'n'feel
  • works on both Qt5/Qt6 using native Status components
  • some smaller fixes/alignments to the underlying StatusDialog
  • add a dedicated StoryBook page

Fixes #17562

Affected areas

StatusMessageDialog

Architecture compliance

Screenshot of functionality (including design for comparison)

  • I've checked the design and this PR matches it

image

@caybro caybro linked an issue Mar 13, 2025 that may be closed by this pull request
[Qt 5->6]: Create a StatusMessageDialog wrapper component for both qt5 and qt6 versions #17562
Closed
@status-im-auto
Copy link
Member

status-im-auto commented Mar 13, 2025 •
edited
Loading

Jenkins Builds

Click to see older builds (7)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ a357686 #1 2025-03-13 16:53:15 ~8 min tests/nim 📄log
✔️ a357686 #1 2025-03-13 16:53:42 ~9 min macos/aarch64 🍎dmg
✔️ a357686 #1 2025-03-13 16:57:54 ~13 min tests/ui 📄log
✔️ a357686 #1 2025-03-13 17:01:39 ~17 min macos/x86_64 🍎dmg
✔️ a357686 #1 2025-03-13 17:06:32 ~22 min linux/x86_64 📦tgz
✔️ a357686 #1 2025-03-13 17:06:40 ~22 min linux-nix/x86_64 📦tgz
✔️ a357686 #1 2025-03-13 17:08:13 ~23 min windows/x86_64 💿exe
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 5e88968 #2 2025-03-14 10:55:01 ~6 min macos/aarch64 🍎dmg
✔️ 5e88968 #2 2025-03-14 10:57:12 ~9 min tests/nim 📄log
✔️ 5e88968 #2 2025-03-14 11:00:45 ~12 min macos/x86_64 🍎dmg
✔️ 5e88968 #2 2025-03-14 11:01:32 ~13 min tests/ui 📄log
✔️ 5e88968 #2 2025-03-14 11:08:03 ~20 min linux-nix/x86_64 📦tgz
✔️ 5e88968 #2 2025-03-14 11:08:18 ~20 min linux/x86_64 📦tgz
✔️ 5e88968 #2 2025-03-14 11:10:20 ~22 min windows/x86_64 💿exe
✔️ b00d367 #3 2025-03-17 09:29:24 ~7 min macos/aarch64 🍎dmg
✔️ b00d367 #3 2025-03-17 09:30:37 ~8 min tests/nim 📄log
✔️ b00d367 #3 2025-03-17 09:35:03 ~13 min macos/x86_64 🍎dmg
✔️ b00d367 #3 2025-03-17 09:35:25 ~13 min tests/ui 📄log
✔️ b00d367 #3 2025-03-17 09:42:49 ~20 min linux/x86_64 📦tgz
✔️ b00d367 #3 2025-03-17 09:43:00 ~21 min linux-nix/x86_64 📦tgz
✔️ b00d367 #3 2025-03-17 09:44:37 ~22 min windows/x86_64 💿exe

@caybro caybro force-pushed the 17562-qt-5-6-statusmessagedialog branch from a357686 to 5e88968 Compare March 14, 2025 10:47
@caybro caybro marked this pull request as ready for review March 14, 2025 11:00
@caybro caybro requested review from micieslak, alexjba and a team as code owners March 14, 2025 11:00
@caybro caybro requested a review from noeliaSD March 14, 2025 11:01
caybro added 2 commits March 17, 2025 10:20
@caybro
feat(StatusQ): create a standard StatusMessageDialog component
82bf386 
- (almost) a dropin replacement for the QtQuick.1 `MessageDialog`,
preserving the same API
- based on `StatusDialog` to get the default look'n'feel
- works on both Qt5/Qt6 using native Status components
- some smaller fixes/alignments to the underlying StatusDialog
- add a dedicated StoryBook page

Fixes #17562
@caybro
feat(StatusMessageDialog): get rid of MessageDialog
b00d367 
- and replace it with `StatusMessageDialog`
@caybro caybro force-pushed the 17562-qt-5-6-statusmessagedialog branch from 5e88968 to b00d367 Compare March 17, 2025 09:21
micieslak
micieslak approved these changes Mar 17, 2025
View reviewed changes
Copy link
Member

@micieslak micieslak left a comment

Choose a reason for hiding this comment

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

Looks good, works fine!

@caybro caybro linked an issue Mar 18, 2025 that may be closed by this pull request
[Qt 5->6]: Review usage of QtQuick.Dialogs 1.x in ChatMessagesView #17533
Closed
@caybro caybro mentioned this pull request Mar 18, 2025
Closed
@caybro caybro linked an issue Mar 18, 2025 that may be closed by this pull request
[Qt 5->6]: Review usage of QtQuick.Dialogs 1.x in CreateCategoryPopup #17536
Closed
This was referenced Mar 18, 2025
Closed
Closed
@caybro caybro linked an issue Mar 18, 2025 that may be closed by this pull request
[Qt 5->6]: Review usage of QtQuick.Dialogs 1.x in CommunityColumnView #17539
Closed
@caybro caybro mentioned this pull request Mar 18, 2025
Closed
@caybro caybro linked an issue Mar 18, 2025 that may be closed by this pull request
[Qt 5->6]: Review usage of QtQuick.Dialogs 1.x in CommunitySettingsView #17540
Closed
@caybro caybro mentioned this pull request Mar 18, 2025
Closed
@caybro caybro linked an issue Mar 18, 2025 that may be closed by this pull request
[Qt 5->6]: Review usage of QtQuick.Dialogs 1.x in RenameAccountModal #17543
Closed
noeliaSD
noeliaSD approved these changes Mar 19, 2025
View reviewed changes
Copy link
Contributor

@noeliaSD noeliaSD left a comment

Choose a reason for hiding this comment

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

Nice job!!
Just to add something.. I'd add a bit more padding in the buttons text since they seem to be too tight.. But that's maybe something for the designers. Up to you!
Screenshot 2025-03-19 at 10 36 39

@caybro
Copy link
Member Author

caybro commented Mar 19, 2025

Nice job!! Just to add something.. I'd add a bit more padding in the buttons text since they seem to be too tight.. But that's maybe something for the designers. Up to you! Screenshot 2025-03-19 at 10 36 39

Ah yeah, totally doable but since there's really no design on this 🤷 😄

@caybro
Copy link
Member Author

caybro commented Mar 19, 2025

Nice job!! Just to add something.. I'd add a bit more padding in the buttons text since they seem to be too tight.. But that's maybe something for the designers. Up to you! Screenshot 2025-03-19 at 10 36 39

Ah yeah, totally doable but since there's really no design on this 🤷 😄

Oh... padding, not spacing! Hmmm strange, they don't appear as tight over here so it must be sth macOS specific? I'm using a regular StatusButton here with nothing special

@caybro caybro merged commit 6999459 into master Mar 19, 2025
9 checks passed
@caybro caybro deleted the 17562-qt-5-6-statusmessagedialog branch March 19, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@micieslak micieslak micieslak approved these changes

@noeliaSD noeliaSD noeliaSD approved these changes

@alexjba alexjba Awaiting requested review from alexjba alexjba is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@caybro @status-im-auto @micieslak @noeliaSD