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] webcomponents everywhere, media field #17740

Closed
wants to merge 6 commits into from
Closed

[4.0] webcomponents everywhere, media field #17740

wants to merge 6 commits into from

Conversation

dgrammatiko
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

  • Move the media field to web components
  • Original functionality
  • Provides more freedom to do do something more with this ultra standard Bootstrap field

REQUESTING FOR SOME BETTER DESIGN

If any designer comes along this PR and they feel that this part needs some redesign love, please do help and provide some mockups

Testing Instructions

Edit an article and change the intro and full text images

Expected result

Same functionality as 3.x

Actual result

screen shot 2017-08-28 at 11 18 32

screen shot 2017-08-28 at 11 18 58

screen shot 2017-08-28 at 11 18 48

Documentation Changes Required

NOPE

@dgrammatiko dgrammatiko changed the title webcomponents everywhere, media field [4.0] webcomponents everywhere, media field Aug 28, 2017
basepath="<?php echo JUri::root(); ?>"
rootfolder="<?php echo ComponentHelper::getParams('com_media')->get('file_path', 'images'); ?>"
url="<?php echo $url; ?>"
modalcont=".modal"
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this one? Can it have an easier to understand name?

Copy link
Contributor

Choose a reason for hiding this comment

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

guessing from line 106 it should be "modalcontainer" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually my naming convention is bad. I will change all these to hyphen separated words: basepath -> base-path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, attribute names back to kebab case.
Also fix the stretched preview image
add (hardcoded ATM) support for more media types
and prepared for the changes in the media manager

@brianteeman
Copy link
Contributor

Looks like you have a bug ;)
screenshotr14-43-27

@dgrammatiko
Copy link
Contributor Author

It's this line: type="image" <? // @TODO add this attribute to the field in order to use it for all media types ?> should be type="image" <? /* @TODO add this attribute to the field in order to use it for all media types */ ?>

@@ -76,6 +76,12 @@
{
$src = JText::_('JLIB_FORM_MEDIA_PREVIEW_EMPTY');
}

if ($showAsTooltip) {
Copy link
Contributor

Choose a reason for hiding this comment

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

	$showPreview = 'static';

	if ($showAsTooltip)
	{
		$showPreview = 'tooltip';
	}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Re assigning a variable is slower than an if/else but anyways I did it

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on e65c4c1


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

@Fedik
Copy link
Member

Fedik commented Sep 30, 2017

the field seems works, but I cannot see any image in modal,
maybe because com_media changes

@dgrammatiko
Copy link
Contributor Author

This is already merged, from the new media manager repo

@dgrammatiko dgrammatiko closed this Oct 8, 2017
@dgrammatiko dgrammatiko deleted the §4.0-dev-field-media branch November 21, 2017 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants