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.

Sign up for GitHub

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

Jump to bottom

BUGFIX: Fix mediabrowser regressions, improve configuration #1305

Merged
merged 46 commits into from Jan 24, 2017
Merged

BUGFIX: Fix mediabrowser regressions, improve configuration #1305

merged 46 commits into from Jan 24, 2017

Conversation

Sebobo
Copy link
Member

@Sebobo Sebobo commented Dec 10, 2016 •
edited by kdambekalns

This acts on feedback after merge of #159

Closes #1284

Things to be worked on

  • Add useful description to package
  • Clean up the two AssetController classes
  • Auto include routes
  • Fix requestPatterns regression
  • Fix whitespace regression document breadcrumb
  • Move translation labels to media browser package
  • Move the stylesheets to the new package
  • Move thumbnail configuration the Neos.Neos
  • Upload broken is user select a tag before upload (affect only master, confirmed by @gerhard-boden see https://neos-project.slack.com/archives/team-unicorn/p1481721419000067)
  • Drag'n drop to Collection / Tag don't work

Infrastructure

  • Sync CrowdIn (translation move, see 94ff769)
  • Update Jenkins build script to build CSS/JS

@Sebobo Sebobo added this to the 3.0 milestone Dec 10, 2016
@mention-bot
Copy link

@Sebobo, thanks for your PR! By analyzing the history of the files in this pull request, we identified @radmiraal, @aertmann and @johannessteu to be potential reviewers.

@kdambekalns kdambekalns changed the title Fix mediabrowser regressions and improve configuration BUGFIX: Fix mediabrowser regressions, improve configuration Dec 10, 2016
@Sebobo Sebobo mentioned this pull request Dec 11, 2016
Merged
@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016

Hi @Sebobo why this is WIP and where did you need help ?

@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016 •
edited

With this change Tagging assets return an error

@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016

Issue with translations

@Sebobo
Copy link
Member Author

Sebobo commented Dec 14, 2016

@dfeyer I started working on the feedback of Aske in #159. But I don't have a good idea or knowledge for everything as I was not involved in the initial implemenation of the change.
There are some issues left. Would be great if could check the feedback at the end of #159 and we can then discuss who of us does what and how :)

@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016

@Sebobo OK so look like a lots of places need some attentions :)

@Sebobo
Copy link
Member Author

Sebobo commented Dec 14, 2016

@dfeyer I updated the description with a todo list

@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016

Translation are moved and flash message are now displayed correctly, but wait @kdambekalns feedback on how to sync with CrowdIn

@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016

Ok I continue on this one this afternoon

dfeyer
dfeyer previously requested changes Dec 14, 2016
View reviewed changes
Copy link
Contributor

@dfeyer dfeyer left a comment

Choose a reason for hiding this comment

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

Don't merge this one, need a bit more love

@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016

@aertmann Can you give us more informations about the regression with the Policy (see #159 (comment)) ?

@dfeyer
Copy link
Contributor

dfeyer commented Dec 14, 2016

Move thumbnail generation and configuration

With b97140d the thumbnail preset configuration are back the the Neos package. Not sure what you mean by "generation" ?

@aertmann
Copy link
Contributor

Hey started reviewing this one and found a bunch of things and ended up spending 12 hours on it 😕

@Sebobo: Could you merge Sebobo#1 into this one and then review it again? Thanks

@aertmann aertmann mentioned this pull request Jan 24, 2017
Merged
@Sebobo
Merge pull request #1 from aertmann/Sebobo-fix-mediabrowser-regressions
67331be 
Refactoring and fixing of various issues and merge conflicts
@Sebobo
Copy link
Member Author

Sebobo commented Jan 24, 2017

@aertmann wow, that's quite a bit of stuff.
I merged it and will later check it out. We really have to get this done at some point...

aertmann
aertmann reviewed Jan 24, 2017
View reviewed changes
Neos.Media.Browser/Classes/Domain/Session/BrowserState.php
@@ -27,7 +27,7 @@ class BrowserState
'activeTag' => null,
'view' => 'Thumbnail',
'sortBy' => 'Modified',
'sortDirection' => 'ASC',
'sortDirection' => 'DESC',
Copy link
Contributor

Choose a reason for hiding this comment

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

same as #1392

@aertmann
Copy link
Contributor

@Sebobo: yeah a bit of a doozy, turned into a little monster

just noticed two more things, will push fixes

thanks for merging it

aertmann
aertmann approved these changes Jan 24, 2017
View reviewed changes
Copy link
Contributor

@aertmann aertmann left a comment

Choose a reason for hiding this comment

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

tested pretty thoroughly

aertmann
aertmann approved these changes Jan 24, 2017
View reviewed changes
Copy link
Contributor

@aertmann aertmann left a comment

Choose a reason for hiding this comment

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

Excellent! Great work guys!

@Sebobo Sebobo merged commit 4800fa5 into neos:3.0 Jan 24, 2017
@Sebobo Sebobo deleted the fix-mediabrowser-regressions branch January 24, 2017 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dfeyer dfeyer dfeyer approved these changes

@aertmann aertmann aertmann approved these changes

@kitsunet kitsunet Awaiting requested review from kitsunet

@gerhard-boden gerhard-boden Awaiting requested review from gerhard-boden

Assignees
No one assigned
Labels
!!! Next major P: Media T: JS T: PHP
Projects
No open projects
Neos 3.0 & Flow 4.0 Release Board
Done
Milestone
3.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@Sebobo @mention-bot @dfeyer @robertlemke @aertmann @kdambekalns