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

Module preview styling #22272

Merged
merged 3 commits into from
Sep 22, 2018
Merged

Module preview styling #22272

merged 3 commits into from
Sep 22, 2018

Conversation

ciar4n
Copy link
Contributor

@ciar4n ciar4n commented Sep 20, 2018

Pull Request for Issue # .

Summary of Changes

General styling. Most notable difference is moving positioning from absolute to relative.

Testing Instructions

Open frontend with module position preview... https://docs.joomla.org/Finding_module_positions_on_any_given_page

Before

image

After

image

Documentation Changes Required

@brianteeman
Copy link
Contributor

Before this pr the module preview would display the position even if it is not active on the page. From your screenshot it looks like you had to create a module in order for it to show. ??

This is much more readable but personally I miss the outline of the position

@ciar4n
Copy link
Contributor Author

ciar4n commented Sep 20, 2018

Nothing has really changed except the styling. Previously when the module was not published, the module position was also echoed as module content. It was not visible because it was covered by the module details (position and style).

I can add back in the outline. My reasoning for removing the outline was that the module width should be evident by the width of the details block. The height equally evident by the following module position.

@brianteeman
Copy link
Contributor

I have found the module outline is useful when the template hardcodes something. For example some templates hardcode the search but keep a module nearby

@ciar4n
Copy link
Contributor Author

ciar4n commented Sep 20, 2018

Something like this suitable?...

image

@brianteeman
Copy link
Contributor

This is the extra text I was referring to. It is confusing because it is shown only if the module position is empty

home

I understand that it was there before and that you couldnt see it - but now that you can see it, it looks confusing

@brianteeman
Copy link
Contributor

Just saw your new screenshot. Is the styling your own or the default styling for that position?

@ciar4n
Copy link
Contributor Author

ciar4n commented Sep 20, 2018

I have removed the content = position if the module is not published. Kittens die when I start diving to deep in to php so might be a good idea if someone could double check that commit.

@brianteeman
Copy link
Contributor

Thanks - looks much better without the module name visible

@ciar4n
Copy link
Contributor Author

ciar4n commented Sep 20, 2018

That styling would be separate to the module styling. Similar to the previous outline.

@brianteeman
Copy link
Contributor

That would be great then - thank you

@ciar4n
Copy link
Contributor Author

ciar4n commented Sep 20, 2018

Module outline added as seen in previous screenshot

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on 6efa100


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

1 similar comment
@Quy
Copy link
Contributor

Quy commented Sep 20, 2018

I have tested this item ✅ successfully on 6efa100


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

@Quy
Copy link
Contributor

Quy commented Sep 20, 2018

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 20, 2018
@mbabker mbabker added this to the Joomla 3.9.0 milestone Sep 22, 2018
@mbabker mbabker merged commit 1c3ca6c into joomla:staging Sep 22, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 22, 2018
@ciar4n ciar4n deleted the display-mod-pos branch September 24, 2018 16:15
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.

5 participants