Skip to content

[4.2] Media manager improvements#39409

Merged
wilsonge merged 3 commits intojoomla:4.2-devfrom
dgrammatiko:4.2-dev_improve_mm
Dec 28, 2022
Merged

[4.2] Media manager improvements#39409
wilsonge merged 3 commits intojoomla:4.2-devfrom
dgrammatiko:4.2-dev_improve_mm

Conversation

@dgrammatiko
Copy link
Copy Markdown
Contributor

@dgrammatiko dgrammatiko commented Dec 13, 2022 •
edited
Loading

Pull Request for Issue # .

Summary of Changes

  • add another script in the list of package.json scripts for compiling MM for development (dev tools friendly)
  • add the package dotenv that is required for the above
  • replace the abandoned vuex-persistedstate with vuex-persist
  • minor CS on the entry of the app

Testing Instructions

  • Run the commands npm ci, npm run build:com_media and npm run build:com_media:dev and verify that media manager works on all of them
  • Create an article and select an intro image (preferably one that is inside a deep nested folder)
  • Observe that the localSession has a value for the joomla.mediamanager
  • Try to select an image for the full image input and verify that the media manager opened in the previous selected folder (ie the one you chose for the intro image)

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

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

The new command is documented in the build/README.md

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.2-dev labels Dec 13, 2022
@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented Dec 15, 2022

We definitely need to swap the persistence plugin out. I also think @laoneo probably should give this a good test against DP Media :)

@ceford
Copy link
Copy Markdown
Contributor

ceford commented Dec 16, 2022

I have tested this item ✅ successfully on 946a99f

I was not sure about the sequence to apply the npm commands - but the media manager worked after npm ci and after npm ci + both others. I could see local session change as different folders were selected. The full image folder opened either with the folder containing the existing image or with the folder from the session storage - is that right. Complicated, but I could not see anything wrong.

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

@dgrammatiko
Copy link
Copy Markdown
Contributor Author

dgrammatiko commented Dec 16, 2022

@ceford thanks for testing this.

Complicated,

Yeah it's weird but somehow we needed to keep in memory the state of the last visited folder and localSession was a good solution. FWIW I'm not changing anything in the logic of the media manager here, I'm just swapping a plugin for another one

@viocassel
Copy link
Copy Markdown
Contributor

viocassel commented Dec 28, 2022

I have tested this item ✅ successfully on 946a99f

👍

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

@richard67
Copy link
Copy Markdown
Member

richard67 commented Dec 28, 2022

RTC

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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 28, 2022
@wilsonge wilsonge merged commit bfa96c5 into joomla:4.2-dev Dec 28, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 28, 2022
@wilsonge
Copy link
Copy Markdown
Contributor

wilsonge commented Dec 28, 2022

Two good tests and seeing a thumbs up from @laoneo so merging. Thanks!

@wilsonge wilsonge added this to the Joomla! 4.2.7 milestone Dec 28, 2022
@dgrammatiko dgrammatiko deleted the 4.2-dev_improve_mm branch December 28, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Milestone

Joomla! 4.2.7

Development

Successfully merging this pull request may close these issues.

6 participants

@dgrammatiko @wilsonge @ceford @viocassel @richard67 @joomla-cms-bot