Skip to content

Remove showModal to replace it by css to not block windows bar actions#441

Merged
AerunDev merged 9 commits intodevelopfrom
fix-43-dialog-show-modal-removed
Mar 7, 2025
Merged

Remove showModal to replace it by css to not block windows bar actions#441
AerunDev merged 9 commits intodevelopfrom
fix-43-dialog-show-modal-removed

Conversation

@AntoinePoree
Copy link
Collaborator

@AntoinePoree AntoinePoree commented Feb 1, 2025

Remove showModal to replace it by css to not block window top bar

Thank you for your contribution to the Pokémon Studio repo.

Before submitting this PR into the develop branch, please make sure:

  • Your code builds clean without any errors or warnings
  • You are following the Code guidelines
  • You tested your code to make sure it does what it is supposed to do

Description

Remove the showModel so now, when a dialog is open, u can pressed button in top of app, like reduce, fullscreen or close

Note before testing

If u make changes on a dialog and try to close the app, a dialog will open for save the app

Tests to perform

Test dialogs over application to be sure thats good ! ;)

Closes #43

@AntoinePoree AntoinePoree self-assigned this Feb 1, 2025
Copy link
Collaborator

@Aelysya Aelysya left a comment

Choose a reason for hiding this comment

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

Tout semble fonctionner nickel, c'est bon pour moi !

@Palbolsky
Copy link
Collaborator

J'ai testé ce qu'à dit Yuri et il y a bien une régression. En spammant TAB on peut se balader en dehors de l'éditeur.

La fonction showModal garantit d'ouvrir un dialog, qui par définition ignore tout ce qui est en dehors et c'est ce dont on a besoin donc je ne pense pas qu'on puisse supprimer la fonction.

La solution que j'avais essayé était de redéfinir une Titlebar dans la dialog pour qu'on puisse l'utiliser, mais j'avais des problèmes de css : la Titlebar ne se plaçait pas correctement mais elle était fonctionnelle.

Copy link
Collaborator

@AerunDev AerunDev left a comment

Choose a reason for hiding this comment

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

Pour ma part tout est bon, on a bien un retour de la possibilité d'utiliser la barre de l'application même avec une Dialog ouverte, et on a bien le TAB / SHIFT + TAB qui réagissent comme attendu en ne sortant pas de la Dialog.

Est-ce qu'on a besoin de tester sur MacOS / Linux si pas de régression ou c'est seulement Windaube related ?

Pour la partie technique je vous laisse me dire si c'est bon pour vous et si oui, je merge ! Bien joué @AntoinePoree 😄

@AerunDev AerunDev changed the title feat(dialog): remove showModal to replace it by css to not block wind… Remove showModal to replace it by css to not block windows bar actions Feb 3, 2025
Copy link
Collaborator

@Palbolsky Palbolsky left a comment

Choose a reason for hiding this comment

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

J'ai trouvé une nouvelle régression. Le MultiLineInput ne se redimensionne plus correctement.
Résultat obtenu à l'ouverture de l'éditeur :

image

Attendu :

image

Copy link
Collaborator

@AerunDev AerunDev left a comment

Choose a reason for hiding this comment

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

Je l'ai précisé sur Discord mais autant centraliser ici : tout est nickel à présent pour les TextArea, mais j'ai l'impression qu'on a une régression sur la modale de traduction, accessible depuis l'icône de traduction dans certains TextArea.
On ne peut plus utiliser TAB pour parcourir les inputs.

@AntoinePoree
Copy link
Collaborator Author

En effet, j'ai pris en compte vos retours.

En prime, voici ce que j'ai aussi corrigé:

  • Dans la globalité, le shift tab est celui qui rencontrait le + de soucis
  • il y avait un soucis sur les modals qui avait des inputs non visible, qui empechait les focus de fonctionner
  • Lorsque vous atteignez le dernier ou premier élément focusable avec tab ou shift tab, le focus va disparaitre et réapparaitre en fonction de comment vous utiliser la tabulation, ce qui permet d'avoir un déplacement plus fluide et sain
  • Sur la modale centrer, pour delete un element par exemple, il est désormais impossible de sortir du focus de la modal avec tab et shift tab

Je vous pris de bien retester et re-vérifier le code, celui ci a un peu changé depuis les dernieres suites à des améliorations pour gérer tout les cas que j'ai pu avoir sur l'application.
Merci d'avance 😇

Copy link
Collaborator

@AerunDev AerunDev left a comment

Choose a reason for hiding this comment

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

Hello @AntoinePoree ! 😄
Je viens de tester ta PR, et globalement ça fait bien le taff.
Je te pose ici deux questions au cas où :

  • Utiliser Tab ou Shift Tab dans la dialog qui apparaît si on a des éléments non sauvegardés amène le focus en dehors de la dialog. Est-ce un oubli ou est-ce que c'est trop complexe à gérer maintenant ?
  • Tu dis dans ton commentaire "Lorsque vous atteignez le dernier ou premier élément focusable avec tab ou shift tab, le focus va disparaitre et réapparaitre en fonction de comment vous utiliser la tabulation, ce qui permet d'avoir un déplacement plus fluide et sain", tu parles bien du fait qu'on a un cran entre le premier et le dernier élément qui est sans focus dans les modales d'édition / création ?

Copy link
Collaborator

@Palbolsky Palbolsky left a comment

Choose a reason for hiding this comment

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

GG pour la tabulation, ça fonctionne correctement.

Par contre quand je fais CTRL+A quand un éditeur est ouvert, même les éléments en dehors de l'éditeur peuvent être copiés, alors que ce n'était pas le cas avant.

Je ne mets pas en Request changes car ça n'empêche pas le bon fonctionnement, mais c'est une régression.

@AntoinePoree
Copy link
Collaborator Author

@AerunDev

  • Le premier point est visiblement un oubli, je ne l'ai tout bonnement pas vu ><
  • C'est ça, je parle de ce fameux cran qui est sans focus pour l'utilisateur

@Palbolsky

  • J'avoue n'avoir pas tester les autres, et le CTRL +A peut etre beaucoup utilisé dans la modal de traduction, je pense que cela mérite une revue de ma part pour délivré une bonne qualité à l'utilisateur

Merci pour ces retours messieurs 😇

Copy link
Collaborator

@Palbolsky Palbolsky left a comment

Choose a reason for hiding this comment

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

C'est bon pour moi. GG :)

@AerunDev AerunDev merged commit b710105 into develop Mar 7, 2025
2 checks passed
@AerunDev AerunDev deleted the fix-43-dialog-show-modal-removed branch March 7, 2025 22:04
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.

The actions of the Windows bar are not available when the editing modal is opened

4 participants