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

[4.0] Media field fix #30455

Merged
merged 9 commits into from
Aug 25, 2020
Merged

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Aug 23, 2020

Pull Request for Issue #30453 .

Summary of Changes

  • Bug fix
  • Remove duplicate bindings

Testing Instructions

  • Download and install the module from @Razzo1987
  • Create a Swiper Slider module
  • Go to "Options" tab
  • Select a Background Image
  • Try to select a different image
  • Remove the image with the X button
  • Try to select an image
  • add more fields and repeat the previous steps

Actual result BEFORE applying this Pull Request

Modal doesn't open as expected

Expected result AFTER applying this Pull Request

Modal opens as expected

Documentation Changes Required

Nope

PS a note here:

  • All Bootstrap init functions need a similar approach, so whenever a Bootstrap component is added programmatically it can be initialised.
  • This is largely unneeded if all the components were custom elements. Custom elements have lifecycle events and therefore the initialisation can happen on the connectedCallback, no extra fuzz, they just work...

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Aug 23, 2020
@infograf768
Copy link
Member

I have tested this item ✅ successfully on 79c4e01


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30455.

@Razzo1987
Copy link
Contributor

Work correctly on the custom module
Work correctly on my module with 2 different image
Do not work on my module with repeatable fields within an image 😢

@infograf768
Copy link
Member

@Razzo1987
Please post here your module zip so that we can test.

@Razzo1987
Copy link
Contributor

Razzo1987 commented Aug 23, 2020

It is in development: 4.zip

you can find a tab "slides". Inside you can find a repeatable filed with images:
image

- Cleanup Media field
- Expose initilazation method of Bootstrpa Modlas to a Joomla.Bootstrap object
- Initilise Modlas on the lifecycle of the Custom Element
…ms into 4.0-dev_media_field

* '4.0-dev_media_field' of github.com:dgrammatiko/joomla-cms:
  typo, (well nothing new)

# Conflicts:
#	build/media_source/system/js/fields/joomla-field-media.w-c.es6.js
@dgrammatiko
Copy link
Contributor Author

@infograf768 @Razzo1987 please retest using @Razzo1987 's module

@Quy
Copy link
Contributor

Quy commented Aug 23, 2020

Edit Swiper Slider module:

Uncaught TypeError: Joomla.Bootstrap is undefined
joomla-field-media.js:231:11

@dgrammatiko
Copy link
Contributor Author

@Quy did you run npm install?

@Quy
Copy link
Contributor

Quy commented Aug 23, 2020

I have tested this item ✅ successfully on 50a892a


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30455.

@Razzo1987
Copy link
Contributor

Razzo1987 commented Aug 23, 2020

I'm testing with

Joomla! 4.0.0-beta4-dev+pr.30455
These packages were built based on commit 50a892acda4a8d677fe50d02b995c535ddc0e100 included in pull request 30455
These packages were built on Sun Aug 23 16:30:38 UTC 2020

from https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/30455/downloads/34914
and doesn't work

@dgrammatiko
Copy link
Contributor Author

@dgrammatiko
Copy link
Contributor Author

@Razzo1987 Just realised that the last commit was not processed, ask someone with Drone access to restart the task

@Razzo1987
Copy link
Contributor

@Quy
Copy link
Contributor

Quy commented Aug 23, 2020

@Razzo1987 Drone restarted. Please try again.

@Razzo1987
Copy link
Contributor

Doesn't work

Joomla! 4.0.0-beta4-dev+pr.30455
These packages were built based on commit 50a892acda4a8d677fe50d02b995c535ddc0e100 included in pull request 30455
These packages were built on Sun Aug 23 17:59:30 UTC 2020

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Aug 23, 2020

Doesn't work

Well, the drone was restarted but there is no new zip file (check the time) so basically you've just tested an earlier version of this PR

@Quy
Copy link
Contributor

Quy commented Aug 23, 2020

It is working for me. It includes the latest commit. See:

These packages were built based on commit 50a892a

Please enable Debug System and check the browser's console.

@Quy
Copy link
Contributor

Quy commented Aug 23, 2020

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 50a892a

Works fine.

Question: could this patch have an impact on other modals (xtd)?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30455.

@dgrammatiko
Copy link
Contributor Author

Question: could this patch have an impact on other modals (xtd)?

I doubt it will affect anything, basically what was done here is to expose the init functionality so it can be used by devs whenever adding programmatically modals. This is needed because all the components using the Bootstrap js need initialisation but custom elements don't need that kind of pampering, which for me is an improved DX.

@infograf768
Copy link
Member

Trust you. ;) RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/30455.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 24, 2020
@infograf768 infograf768 added this to the Joomla 4.0 milestone Aug 24, 2020
@Razzo1987
Copy link
Contributor

Razzo1987 commented Aug 25, 2020

EDITED!
it's OK!
I have cache problems 😄

@infograf768 infograf768 merged commit 6992a2e into joomla:4.0-dev Aug 25, 2020
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 25, 2020
@infograf768
Copy link
Member

Tks!

@dgrammatiko dgrammatiko deleted the 4.0-dev_media_field branch August 25, 2020 10:19
drmenzelit pushed a commit to drmenzelit/joomla-cms that referenced this pull request Aug 31, 2020
* fix

* typo, (well nothing new)

* Allow media fields in subforms

- Cleanup Media field
- Expose initilazation method of Bootstrpa Modlas to a Joomla.Bootstrap object
- Initilise Modlas on the lifecycle of the Custom Element

* Paleolithic js mode only in the legacy…

* CS

* Proper check

* Pffff

* Update joomla-field-media.w-c.es6.js
infograf768 added a commit that referenced this pull request Sep 4, 2020
* First step to make newsflash horizontal

* [4.0] Fix Alert double error display in Installation

* seperating multiple messages with border top

* Increase minimum length

* Fix meter displaying complete message prematurely

* Add strenthmeter attribute to installation

* Change meter values to display green when password requirements are met

* [4.0] {Cassiopea] Implementing Password length/meter

* [4.0] Fix Fieldset description display (#30435)

* [4.0] Url Language Code is not a prefix (#30440)

* [4.0] URL LANGUAGE CODE is not a prefix

* modified desc as suggested

* [4.0] Implementing display of fieldset descriptions for fieldset
children

* Taking off description

* scss cs

* use @Quy suggestion

* file cs

* [4.0] Media field fix (#30455)

* fix

* typo, (well nothing new)

* Allow media fields in subforms

- Cleanup Media field
- Expose initilazation method of Bootstrpa Modlas to a Joomla.Bootstrap object
- Initilise Modlas on the lifecycle of the Custom Element

* Paleolithic js mode only in the legacy…

* CS

* Proper check

* Pffff

* Update joomla-field-media.w-c.es6.js

* [4.0] Convert icon used for jgrid defaults to fas fa- (#30464)

* make icon-circle use fa-circle

* allows for including icon- in call

* move fa-fw to last class to fit convention (#30469)

* [4.0] Long labels wrapping (#30474)

When we have fields displayed in columns with the label displayed above the input there is a width limit of 240px on the label.

This results in long labels wrapping when it is not needed as the column is wider than 240px.

In english we dont see this is many places but the easiest is in the __configure edit screen_  fields

This simple PR changes the label width to 100% when the fields are displayed in columns like this.

As this is an scss change you will need to `node build.js --compile-css` in order to test

### Before

### After

* Fix mod_articles_latest (#30459)

* [4.0] Add a parameter for "back-to-top" button in Cassiopeia (#30441)

* [4.0] Add a parameter for back-to-top button in Cassiopeia

Added a parameter to show / hide the back-to-top button in Cassiopeia.
Currently the footer appears when a module has been published on position "footer". And the back-to-top only appears if the footer is visible. Since the footer is part of the grid, it is not easy to move the back-to-top outside the grid.

Now it is possible to decide to have a back-to-top or not. If yes, the footer will be rendered showing the button, independently if there is a module in the position footer.
It is also possible to show only modules (switch back-to-top off) or modules and back-to-top (switch on).

* Added smooth scrolling

Add smooth scrolling taking in account the preferences of the user (reduced motion).
Improve  language file.

* Correct height and position of back-to-top button

* Correct code style

* Correct code style 2

* Change smooth scrolling behavior to take in account reduced motion preferences

* Language file corrected, language string changed to match others

* Update language/en-GB/tpl_cassiopeia.ini

Link instead button

Co-authored-by: Brian Teeman <[email protected]>

* Update templates/cassiopeia/scss/blocks/_global.scss

Comment for smooth scroll

Co-authored-by: Brian Teeman <[email protected]>

Co-authored-by: Brian Teeman <[email protected]>

* fix improper family name. (#30481)

* [4.0] update helptoc (#30490)

* [4.0] update helptoc

Removes the string for the expired cache page that no longer exists and removes it from the index displayed on the left hand side in the help page

* api

* [4.0] Add new permissions-policy to the HTTPHeaders Plugin (#30491)

* add permissions-policy

* add Permissions-Policy to the dropdown

* alpha order

* [4.0] Error when changing status of tagged content items (#30466)

* Correct column name

* Fix changing UCM state

* [4.0] br tag (#30503)

* [4.0] br tag

In Joomla 4 we use `<br>` not `<br />`

* bad grep

* [4.0] Remove old search component (#30506)

* [4.0] Remove old search component

The regular search component is not in J4

This pr removes reference to it in the help system
`administrator/index.php?option=com_admin&view=help`

* revert

* [4.0] Help Dashboard (#30508)

* [4.0] Help Dashboard

This PR makes multiple changes to the help dashboard. In order of importance they are.

- Created a new section "Start Here"
- Moved "Joomla Help" to the "Start Here" section to give it greater priority as it really is the first place you should look for help
- Removed target=_blank from "Joomla Help"
- Moved "Documentation Wiki" from "Resources" to the top of "Additional Help"

* deduplicate

* compass icon

* Moved css for mod_articles_news to media folder

User webAsset Manager to load specific css for module

* Move web asset call to horizontal.php

Co-authored-by: Jean-Marie Simonet <[email protected]>
Co-authored-by: Quy <[email protected]>
Co-authored-by: dGrammatiko <[email protected]>
Co-authored-by: Bear <[email protected]>
Co-authored-by: Brian Teeman <[email protected]>
Co-authored-by: Christiane Maier-Stadtherr <[email protected]>
Co-authored-by: Tobias Zulauf <[email protected]>
Co-authored-by: SharkyKZ <[email protected]>
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
* fix

* typo, (well nothing new)

* Allow media fields in subforms

- Cleanup Media field
- Expose initilazation method of Bootstrpa Modlas to a Joomla.Bootstrap object
- Initilise Modlas on the lifecycle of the Custom Element

* Paleolithic js mode only in the legacy…

* CS

* Proper check

* Pffff

* Update joomla-field-media.w-c.es6.js
sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
* First step to make newsflash horizontal

* [4.0] Fix Alert double error display in Installation

* seperating multiple messages with border top

* Increase minimum length

* Fix meter displaying complete message prematurely

* Add strenthmeter attribute to installation

* Change meter values to display green when password requirements are met

* [4.0] {Cassiopea] Implementing Password length/meter

* [4.0] Fix Fieldset description display (joomla#30435)

* [4.0] Url Language Code is not a prefix (joomla#30440)

* [4.0] URL LANGUAGE CODE is not a prefix

* modified desc as suggested

* [4.0] Implementing display of fieldset descriptions for fieldset
children

* Taking off description

* scss cs

* use @Quy suggestion

* file cs

* [4.0] Media field fix (joomla#30455)

* fix

* typo, (well nothing new)

* Allow media fields in subforms

- Cleanup Media field
- Expose initilazation method of Bootstrpa Modlas to a Joomla.Bootstrap object
- Initilise Modlas on the lifecycle of the Custom Element

* Paleolithic js mode only in the legacy…

* CS

* Proper check

* Pffff

* Update joomla-field-media.w-c.es6.js

* [4.0] Convert icon used for jgrid defaults to fas fa- (joomla#30464)

* make icon-circle use fa-circle

* allows for including icon- in call

* move fa-fw to last class to fit convention (joomla#30469)

* [4.0] Long labels wrapping (joomla#30474)

When we have fields displayed in columns with the label displayed above the input there is a width limit of 240px on the label.

This results in long labels wrapping when it is not needed as the column is wider than 240px.

In english we dont see this is many places but the easiest is in the __configure edit screen_  fields

This simple PR changes the label width to 100% when the fields are displayed in columns like this.

As this is an scss change you will need to `node build.js --compile-css` in order to test

### Before

### After

* Fix mod_articles_latest (joomla#30459)

* [4.0] Add a parameter for "back-to-top" button in Cassiopeia (joomla#30441)

* [4.0] Add a parameter for back-to-top button in Cassiopeia

Added a parameter to show / hide the back-to-top button in Cassiopeia.
Currently the footer appears when a module has been published on position "footer". And the back-to-top only appears if the footer is visible. Since the footer is part of the grid, it is not easy to move the back-to-top outside the grid.

Now it is possible to decide to have a back-to-top or not. If yes, the footer will be rendered showing the button, independently if there is a module in the position footer.
It is also possible to show only modules (switch back-to-top off) or modules and back-to-top (switch on).

* Added smooth scrolling

Add smooth scrolling taking in account the preferences of the user (reduced motion).
Improve  language file.

* Correct height and position of back-to-top button

* Correct code style

* Correct code style 2

* Change smooth scrolling behavior to take in account reduced motion preferences

* Language file corrected, language string changed to match others

* Update language/en-GB/tpl_cassiopeia.ini

Link instead button

Co-authored-by: Brian Teeman <[email protected]>

* Update templates/cassiopeia/scss/blocks/_global.scss

Comment for smooth scroll

Co-authored-by: Brian Teeman <[email protected]>

Co-authored-by: Brian Teeman <[email protected]>

* fix improper family name. (joomla#30481)

* [4.0] update helptoc (joomla#30490)

* [4.0] update helptoc

Removes the string for the expired cache page that no longer exists and removes it from the index displayed on the left hand side in the help page

* api

* [4.0] Add new permissions-policy to the HTTPHeaders Plugin (joomla#30491)

* add permissions-policy

* add Permissions-Policy to the dropdown

* alpha order

* [4.0] Error when changing status of tagged content items (joomla#30466)

* Correct column name

* Fix changing UCM state

* [4.0] br tag (joomla#30503)

* [4.0] br tag

In Joomla 4 we use `<br>` not `<br />`

* bad grep

* [4.0] Remove old search component (joomla#30506)

* [4.0] Remove old search component

The regular search component is not in J4

This pr removes reference to it in the help system
`administrator/index.php?option=com_admin&view=help`

* revert

* [4.0] Help Dashboard (joomla#30508)

* [4.0] Help Dashboard

This PR makes multiple changes to the help dashboard. In order of importance they are.

- Created a new section "Start Here"
- Moved "Joomla Help" to the "Start Here" section to give it greater priority as it really is the first place you should look for help
- Removed target=_blank from "Joomla Help"
- Moved "Documentation Wiki" from "Resources" to the top of "Additional Help"

* deduplicate

* compass icon

* Moved css for mod_articles_news to media folder

User webAsset Manager to load specific css for module

* Move web asset call to horizontal.php

Co-authored-by: Jean-Marie Simonet <[email protected]>
Co-authored-by: Quy <[email protected]>
Co-authored-by: dGrammatiko <[email protected]>
Co-authored-by: Bear <[email protected]>
Co-authored-by: Brian Teeman <[email protected]>
Co-authored-by: Christiane Maier-Stadtherr <[email protected]>
Co-authored-by: Tobias Zulauf <[email protected]>
Co-authored-by: SharkyKZ <[email protected]>
@HRIT-Florian
Copy link
Contributor

HRIT-Florian commented Mar 1, 2021

@infograf768 In current BETA Problem still exists: See #26711
and #28953

@HRIT-Florian
Copy link
Contributor

@dgrammatiko can you check again?

@dgrammatiko
Copy link
Contributor Author

@Didldu-Florian change

&& window.bootstrap.Modal.getInstance(this.modalElement) === undefined) {
to && window.bootstrap.Modal.getInstance(this.modalElement) === null) {

Also if you do a PR please also apply the same fix here:

&& window.bootstrap.Modal.getInstance(this.modal) === undefined) {

@HRIT-Florian
Copy link
Contributor

@dgrammatiko oh, nice, thanks :) That fix works.
But why does no PR exists for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants