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] Dashboard improvements #17203

Merged
merged 3 commits into from Jul 22, 2017
Merged

[4.0] Dashboard improvements #17203

merged 3 commits into from Jul 22, 2017

Conversation

C-Lodder
Copy link
Member

@C-Lodder C-Lodder commented Jul 21, 2017

Pull Request for Issue #14765

Summary of Changes

This PR adds a masonry grid to the dashboard modules, preventing whitespace between columns.

There was also a a cog icon for each module that when clicked, displayed a dropdown with an "Edit" option. Now, upon click, there is no longer a dropdown, but instead it instantly directs you to the edit view.

Screenshot

cpanel

@ghost
Copy link

ghost commented Jul 21, 2017

I have tested this item ✅ successfully on 6835cb2


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

1 similar comment
@ciar4n
Copy link
Contributor

ciar4n commented Jul 21, 2017

I have tested this item ✅ successfully on 6835cb2


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

@ghost
Copy link

ghost commented Jul 21, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 21, 2017
@C-Lodder
Copy link
Member Author

@franz-wohlkoenig please could you remove RTC as I'm going to switch from masonry to a much lighter package.

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 21, 2017
@ghost
Copy link

ghost commented Jul 21, 2017

set Status on "Pending".

@brianteeman
Copy link
Contributor

@C-Lodder wouldnt it be better to use css grid for this instead of a js solution as we dont really need the full scope of masonry here

https://rachelandrew.co.uk/archives/2017/01/18/css-grid-one-layout-method-not-the-only-layout-method/

@dgrammatiko
Copy link
Contributor

@brianteeman I was thinking of that as well...

@ciar4n
Copy link
Contributor

ciar4n commented Jul 21, 2017

In this case a 'masonry' style layout is not possible with CSS Grid alone. At least one that is fluid and will change depending on the size of the modules enabled.

@C-Lodder
Copy link
Member Author

@franz-wohlkoenig sorry, could you re-add RTC as the other library doesn't support different widths

@ghost
Copy link

ghost commented Jul 22, 2017

RTC as stated above.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jul 22, 2017
@wilsonge wilsonge merged commit a938080 into joomla:4.0-dev Jul 22, 2017
@wilsonge wilsonge added this to the Joomla 4.0 milestone Jul 22, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jul 22, 2017
@dgrammatiko
Copy link
Contributor

@wilsonge so we are happy to deliver an 8 year old script :(

@brianteeman
Copy link
Contributor

For once i agree with D!otri

@wilsonge
Copy link
Contributor

If you mean something last released on the 20th April this year yes. If you can come up with a better solution that delivers the same outcome then we can consider it. But I consider this a substantial improvement on what we have now.

@C-Lodder C-Lodder deleted the dashboard2 branch July 22, 2017 18:45
@dgrammatiko
Copy link
Contributor

@wilsonge what is the required outcome?
Why do we need this script?
In fact, is this some agreed solution for the control panel?

Was this decided by some group, or just had 2 RTC...

By the way, if you're about to commit new libraries without any discussion, then the JS group has no relevance and we should just remove it, as it's a wasted time for those that participating there...

@wilsonge
Copy link
Contributor

This PR is by someone in the JS group....

@dgrammatiko
Copy link
Contributor

@wilsonge this script was never mentioned in that group or in any other group AFAIK

@mbabker
Copy link
Contributor

mbabker commented Jul 22, 2017

OK this is getting mildly obnoxious.

@C-Lodder
Copy link
Member Author

I dont mind refactoring and doing something different. As mentioned on skype, this was more of a fallback solution and could be used to help me with the drag n drop.

I did also ask on Skype if you and Ciaran were happy with this. You flagged that fact that we could use Css grid and also the masonry license. Ciaran mentioned css grid couldnt be used and you fou d the license section therefore said, and I quote

scrap that, masonry is ok

Like I said, I'm happy to use something more lightweight.

We tried the other library but it had some width fallbacks

@ciar4n
Copy link
Contributor

ciar4n commented Jul 23, 2017

Why do we need this script?

To avoid scenarios like this...

masonry

Considering the varying nature of these modules (height & width), masonry.js is the only real solution here. 8 years and it is still the go to standard for these layouts. If using it is an issue then we need to either remove the option to customise the module widths or find an alternative concept for the dashboard.

Fix widths for these modules would open up some lighter solutions..

@brianteeman
Copy link
Contributor

I am going to have a play with the suggestion I made above

@ciar4n
Copy link
Contributor

ciar4n commented Jul 23, 2017

@brianteeman You're welcome to try however as stated in your suggestion...

For Masonry what you actually would need is for auto-placement to look at both height and width and be able to create ‘rows’ that also push items up into the area of the row above to fill in gaps. You can get something that looks very much like that if you control the layout by positioning the items. However you then can’t just throw a chunk of content at a grid and let it do the layout - which is what people want in ‘Masonry’.

@dgrammatiko
Copy link
Contributor

As mentioned on skype, this was more of a fallback solution and could be used to help me with the drag n drop.

Probably I don't get your way of implementing things. I would expect the drag and drop to go first and if there is a need to tweak the outcome then do that. Not the other way around. Also there is a PR in the other repo that showcases how dead easy is to implement the drag and drop (at least client side). Obviously we see things from different perspective here

To avoid scenarios like this...

Oh, well you can avoid that with other tricks, if you think a little harder ;)

@C-Lodder
Copy link
Member Author

Yup, I would have done the drag n drop too however as I mentioned yesterday, Im stillt rying to figure out the best approach. Who knows, I may not even have the skill set to do it. With that in mind, this was more of a temorary fallback to fix the module layout being screwed up just incase I fail the DnD.

I remember you mentioning the other repo yesterday, however since this PR, I havent had time to sit down at my computer and look at it. When I get a chance, I will

@brianteeman
Copy link
Contributor

The css is beyond my skillset but both these look doable
https://medium.com/@_jh3y/how-to-pure-css-masonry-layouts-a8ede07ba31a
http://w3bits.com/css-masonry/

Maybe I am missing something as to why we cant use these methods (or similar)

@ciar4n
Copy link
Contributor

ciar4n commented Jul 23, 2017

Oh, well you can avoid that with other tricks, if you think a little harder ;)

I guess until these elusive tricks are implemented this PR gives us a working solution :)

https://medium.com/@_jh3y/how-to-pure-css-masonry-layouts-a8ede07ba31a
http://w3bits.com/css-masonry/

Both of these methods use the column-count property which I mentioned above. This property does not support various column widths. Using count-columns means that we have to have a set number of columns of equal widths so that means removing the option to change the module width.

@dgrammatiko
Copy link
Contributor

@C-Lodder @ciar4n can we step back and answer one vital question here:
do we want user defined layout for the dashboard?

I had the impression that was the aim, am I right?
If so how compatible is masonry with that?

Masonry at it's core is a block randomiser which is the opposite of what we are trying to achieve here. Drag and drop something in masonry and observe the randomiser taking place for the rest of the blocks. How intuitive that will be for someone that wants to layout things in a particular order?

@ciar4n
Copy link
Contributor

ciar4n commented Jul 23, 2017

do we want user defined layout for the dashboard?

Personally I would say yes. That been drag'n'drop would be an added bonus but I would not say essential. For me this resolves a visual issue rather than a gateway to a drag n drop feature.

@dgrammatiko
Copy link
Contributor

For me this resolves a visual issue rather than a gateway to a drag n drop feature.

Right, it does, but it also cancels the module order...

@ciar4n
Copy link
Contributor

ciar4n commented Jul 23, 2017

A multi width masonry layout by it's nature will always kill module order which is why I suspect drag'n'drop may be difficult. Multi width masonry layouts are defined by the size of the modules rather than their order which is why saving such layouts by order number is not really an option IMO (hence the difficulty).

Hopefully I am wrong but as I see it is either fixed module widths or no drag'n'drop.

@dgrammatiko
Copy link
Contributor

@ciar4n implementing a masonry layout on an image gallery is fine, here we might be creating more problems than the ones we think we are fixing! The dashboard is a functional layout, meaning the order of things is essential! I want my analytics stats module in the top not randomly placed wherever is more convenient depending on the screen size!

Anyways, this PR cancels out the module order, if that's the way for the dashboard I'll STFU.

@ciar4n
Copy link
Contributor

ciar4n commented Jul 23, 2017

Order will play a part in setting a masonry layout but I guess the flow is not always predictable. A full width module ordered 1st will always display on top with masonry.js. Now that I think about it although the visual order might not match the actual order, the saved order should always give the same result with masonry.js so maybe there is a dnd workaround.

Regardless I am not saying this is the only solution. I am not against a simpler one.. eg. have a full width header position followed by 2 column positions. Remove the width field and have the modules width determined by its position (bye bye masonry.js). I'm sure there is other solutions. For now as mentioned by @C-Lodder this is 'a temporary fallback to fix the module layout being screwed up'

@dgrammatiko
Copy link
Contributor

I hate to be the bad guy here but this is utterly wrong!

  • Using javascript to solve layout problems should always be our last resort, not the first one!
  • We are breaking Joomla's module order here!
  • We are breaking accessibility (try tab through the rearranged - and obviously more appealing - masonry layout and observe the ping-pong)

If we want to redesign the dashboard we can easily follow the windows tiles example with some small predefined boxes (even with graphs) that are clickable and bring a modal for a more in depth view, or....

@brianteeman
Copy link
Contributor

spam

@joomla joomla deleted a comment from raviksharma90 Jan 21, 2019
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

7 participants