Skip to content

[4.1] Fire change events consistently on the media Field#37652

Closed
dgrammatiko wants to merge 30 commits intojoomla:4.2-devfrom
dgrammatiko:patch-2
Closed

[4.1] Fire change events consistently on the media Field#37652
dgrammatiko wants to merge 30 commits intojoomla:4.2-devfrom
dgrammatiko:patch-2

Conversation

@dgrammatiko
Copy link
Contributor

Pull Request for Issue #37640 .

Summary of Changes

  • Call the Custom Element function setValue instead of manually updating the value

Testing Instructions

  • Edit an article and go to the images tab
  • Open the inspector and inspect the intro image input (the one left to the select button bellow the image)
  • type in the browser's console $0.addEventListener('change', (e) => console.log(e))
  • Select an image, check that a change event was recorded in the console
  • Clear the field, check the recorded event
  • Select another image and check that the event was recorded again

Actual result BEFORE applying this Pull Request

No event on image selection

Expected result AFTER applying this Pull Request

Documentation Changes Required

No this is a bug

dgrammatiko and others added 29 commits December 19, 2021 20:19
Change to use DATE_FORMAT_LC instead of RFC822 to enable translated dates in the notification emails.
English grammar corrections

Co-authored-by: Quy <quy@fluxbb.org>
Co-authored-by: Quy <quy@fluxbb.org>
Adds the missing table caption for the two tables in the multlilangstatus _module_

There is no visual change as the captions are seet to be visually-hidden
Its a subtle change but a correct one as the "read more" is the primary and "title" the secondary function of the option

It also means that all three read more options are structured similarly grammatically
* [4.1] com_media: hardcoded language strings
* Update link.php

the pagnation in has an misuse of the aria-label. i fixed it with two span tags.

* Update layouts/joomla/pagination/link.php

Co-authored-by: Brian Teeman <brian@teeman.net>
### Steps to reproduce the issue
Create a menu of type Articles Category List
Create a menu of type List all contacts in a category

Enable the display filter option in both


### Expected result
the filter field works/displays the same in both


### Actual result Com_content
In com_content there is an option to select what the filter field should be and depending on the filter field the display & functionality in the front end is adjusted.

![image](https://user-images.githubusercontent.com/1296369/160798796-47a9c09e-2f87-48f8-b3f9-a8037a4da032.png)

#### Filter - Author (input)
![image](https://user-images.githubusercontent.com/1296369/160798237-801ff1b7-5b32-46ae-b9fc-c95e8d3af541.png)

#### Filter - Title (input)
![image](https://user-images.githubusercontent.com/1296369/160798366-ff28334a-b471-439e-8506-3b2188305caf.png)

#### Filter - Month(select)
![image](https://user-images.githubusercontent.com/1296369/160798623-f39426d4-92dc-4e47-b89d-be4f037ca8b6.png)

### Actual result com_contact
In com_contact the option is just show/hide
![image](https://user-images.githubusercontent.com/1296369/160799241-017d2fa9-e763-4d2d-ba3c-9043adc3490c.png)

![image](https://user-images.githubusercontent.com/1296369/160800153-c505a4b3-f74e-47e3-aee5-e950ded3943b.png)

### Additional comments
The _problem_ is 
1. com_contacts the form option in the admin is **filter**
2. com_contacts the button in the site is **filter**
3. the placeholder text (and aria-label)  is **search in title** in com_contacts but just **title filer** in com_contact

For a consistent ui this PR changes search in title to title filter

## Result
![image](https://user-images.githubusercontent.com/1296369/160802562-2988b5d9-b35d-442f-8551-5345c46aeb3b.png)
* Unnecessary description for layout

* Added info about deprecated string

* Update administrator/language/en-GB/joomla.ini

Co-authored-by: Allon Moritz <allon.moritz@digital-peak.com>
@joomla-cms-bot joomla-cms-bot added the NPM Resource Changed This Pull Request can't be tested by Patchtester label Apr 25, 2022
@laoneo
Copy link
Member

laoneo commented Apr 25, 2022

Can you rebase this to 4.2?

@dgrammatiko dgrammatiko changed the base branch from 4.1-dev to 4.2-dev April 25, 2022 12:33
@dgrammatiko dgrammatiko requested a review from rdeutz as a code owner April 25, 2022 12:33
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-4.2-dev Unit/System Tests and removed NPM Resource Changed This Pull Request can't be tested by Patchtester labels Apr 25, 2022
@dgrammatiko dgrammatiko changed the base branch from 4.2-dev to 4.1-dev April 25, 2022 12:34
@dgrammatiko dgrammatiko changed the base branch from 4.1-dev to 4.2-dev April 25, 2022 12:35
@dgrammatiko
Copy link
Contributor Author

So 4.1 and 4.2 are not in sync? @laoneo

@richard67
Copy link
Member

@dgrammatiko I think the problem with the branch sync will be solved after the next upmerge from 4.1-dev to 4.2-dev and then update of the branch of this PR to 4.2-dev. As there is an alpha-2 release scheduled for tomorrow, I assume the upmerge will happen just after that.

@Quy Quy removed the PR-4.1-dev label Apr 25, 2022
@dgrammatiko
Copy link
Contributor Author

@richard67 sure but I have no clue why this small bug fix needs to wait few months in order to be (potentially) merged. It's not very helpful for those devs that have a problem with the field right now. Anyways, not my call, just seems not very dev friendly approach

@richard67
Copy link
Member

@richard67 sure but I have no clue why this small bug fix needs to wait few months in order to be (potentially) merged.

@dgrammatiko Maybe you misunderstood or I explained wrong. Tomorrow is 4.2 alpha-2 and after that the upmerge, so your PR can be tested and RTC'ed on the day after tomorrow, I would think.

@richard67
Copy link
Member

@dgrammatiko P.S. Ah .. you mean the rebase decision? Well I don't know why that was made. @laoneo ?

@richard67
Copy link
Member

@laoneo To me this PR seems to be a bug fix, so I don't see why it should go into 4.2. What am I missing?

@dgrammatiko
Copy link
Contributor Author

@richard67 this should be already fixed for 4.2 with #37654, so this isn’t needed anymore

@dgrammatiko dgrammatiko deleted the patch-2 branch April 30, 2022 11:28
@richard67
Copy link
Member

@dgrammatiko Thanks.

@dgrammatiko
Copy link
Contributor Author

Unless the fix is ok for 4.1, then I can redo this PR (2 lines or whatever)

@richard67
Copy link
Member

Unless the fix is ok for 4.1, then I can redo this PR (2 lines or whatever)

@bembelimen I think that's on you to decide.

@dgrammatiko
Copy link
Contributor Author

So I redid it in #37702, feel free to close the PR if 4.2 is fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.