? NPM Resource Changed PR-4.3-dev Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
19 Mar 2023

Pull Request for Issue # .

Summary of Changes

  • uses the Joomla Dialog from #40150 for the fields User and Media

Testing Instructions

  • Apply the patch
  • Run npm ci
  • Check that intro, full text images and author fields are working as before

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

User Field
Screenshot 2023-03-20 at 11 33 33

Media Field
Screenshot 2023-03-20 at 11 33 48

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

avatar joomla-cms-bot joomla-cms-bot - change - 19 Mar 2023
Category Repository NPM Change JavaScript Layout
avatar dgrammatiko dgrammatiko - open - 19 Mar 2023
avatar dgrammatiko dgrammatiko - change - 19 Mar 2023
Status New Pending
avatar dgrammatiko dgrammatiko - change - 19 Mar 2023
Labels Added: ? NPM Resource Changed PR-4.3-dev
20fa94f 19 Mar 2023 avatar dgrammatiko cs
avatar Quy
Quy - comment - 19 Mar 2023

users-dialog

avatar Fedik
Fedik - comment - 20 Mar 2023

Looks good, I still need to test.
Couple notes:
I call the asset "dialog" instead of joomla-dialog, because, well, it joomla already, like we have 'core', 'showon' etc ;)
I would prefer do not to have an extra css, it is very basic, I would do it as part of main template.
For width and height, would use default (defined by css) to keep all popup in consistent size, and set it in script when it need to be diferent.

avatar dgrammatiko
dgrammatiko - comment - 20 Mar 2023

I call the asset "dialog" instead of joomla-dialog, because, well, it joomla already, like we have 'core', 'showon' etc ;)

I renamed them. I think it would be a good idea to have a system/ui folder and get also all the custom elements from the other repo here. I mean if there's a way to move the docs and the tests there's no reason to have multiple repos

I would prefer do not to have an extra css, it is very basic, I would do it as part of main template.

I disagree here. The script should provide some basic styling (mind I didn't copy everything from your PR, only the CE parts) and also the template has the ability to override the assets. Also we're doing that with the alerts and tabs..

For width and height, would use default (defined by css) to keep all popup in consistent size, and set it in script when it need to be diferent.

Removed them

avatar dgrammatiko dgrammatiko - change - 20 Mar 2023
The description was changed
avatar dgrammatiko dgrammatiko - edited - 20 Mar 2023
avatar Fedik
Fedik - comment - 20 Mar 2023

The script should provide some basic styling

That is true, originaly I also wanted to do a separated css, but then I realise that the basic styling already comes with native dialog element ;)
Well, I know it is ugly, but it is usable. There only issue is iframe sizing.

avatar dgrammatiko dgrammatiko - change - 20 Mar 2023
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2023-03-20 16:20:47
Closed_By dgrammatiko
avatar dgrammatiko dgrammatiko - close - 20 Mar 2023

Add a Comment

Login with GitHub to post a comment