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

[RFC] Possible solution for toolbar (3.x) #13230

Closed
wants to merge 1 commit into from
Closed

[RFC] Possible solution for toolbar (3.x) #13230

wants to merge 1 commit into from

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Dec 15, 2016

Pull Request for Issue #12789 .

Summary of Changes

Make the toolbar scrollable e.g.:
dec-15-2016 15-49-15

This is nowhere near production ready!!!!

@PhilETaylor @cpfeifer @C-Lodder @ciar4n


This change is Reviewable

@PhilETaylor

This comment was marked as abuse.

1 similar comment
@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Contributor Author

@PhilETaylor there are not many things that could be done in 3.x and provide a solid solution (without some lag till everybody implement some different behaviour)

@PhilETaylor

This comment was marked as abuse.

@dgrammatiko
Copy link
Contributor Author

@PhilETaylor that's the rubber band effect of safari...

@ciar4n
Copy link
Contributor

ciar4n commented Dec 15, 2016

One concern would be no visible marker that this area is scrollable.

@dgrammatiko
Copy link
Contributor Author

@ciar4n we could do some quick check with javascript and if the div's width is bigger than the view port (user can scroll) we could append some text bellow the toolbar, e.g. scroll to reveal the rest of the buttons

@C-Lodder
Copy link
Member

I'm just about to test properly, but without hijacking the mousewheel, I don't believe this will work with a mouse on Windows. You'll have the manually drag the scrollbar.

@C-Lodder
Copy link
Member

Ok some proper tests:

1. Doesn't work with a mouse on Windows (this will require Javascript)

2. A blank scrollbar appears when it's not required

screeny1

3. The "Help" and "Options" buttons seem to stack on to the next row, even on large screens

screeny2

@dgrammatiko
Copy link
Contributor Author

@C-Lodder not that bad for a quick dirty patch! The main question here is: do we really wanna go this way?
I mean the J4 will introduce something totally different, so does it even worth the effort to try and fix this for 3.x (with this or another possible solution)?

@C-Lodder
Copy link
Member

Not for me to decide on. Best ask the UX team

@PhilETaylor

This comment was marked as abuse.

@C-Lodder
Copy link
Member

@cpfeifer

@ghost
Copy link

ghost commented Dec 15, 2016

I'm here :)

I see where you're going with this, but I'm not sure this is the right solution for J3.

Easy fixes could include adjusting the float / media query on the two buttons. The right float could only be applied on large desktops and that would prevent the break on smaller screens.

Or, in line with the new backend, on smaller screens some of the buttons become a drop down under a single button. @C-Lodder - you know what I mean right?

@C-Lodder
Copy link
Member

Yup, I know what you mean. Either way, will leave this up to UX to decide on and implement if you want. I resigned from all toolbar duties a while back.

@dgrammatiko
Copy link
Contributor Author

@cpfeifer how come adjusting the float that will fix the wrapping problem here?

Toolbar looks ugly and wraps onto two lines with sharedrafts

@ghost
Copy link

ghost commented Dec 15, 2016

@dgt41 - it doesn't necessarily "fix" it, but it allows those two button to behave the same as the others. Those two stick out while the others don't.

@C-Lodder I think that's the way to go here. Consolidate them under a drop down at smaller sizes, leave the most used available as buttons and group the others under one button. Like publish, unpublish, feature, trash etc... - which ones are the most used is debatable but those are the obvious ones. Can you implement this into a PR for us please.

@ciar4n
Copy link
Contributor

ciar4n commented Dec 15, 2016

There is also this.. #12789 (comment)

Buttons will display without the spacing only if the screen isn't wide to accommodate the default.

@dgrammatiko
Copy link
Contributor Author

Consolidate them under a drop down at smaller sizes

Will you envision this to be done automatically, in the client side?

@C-Lodder
Copy link
Member

C-Lodder commented Dec 15, 2016

@cpfeifer - as I said, I've resigned from all toolbar duties. At least until I'm given an absolute final plan of action that has been agreed upon. Putting them into a dropdown on on ALL screen sizes was an absolute nightmare with the Toolbar API being what it is, never mind ONLY smaller screens.

@ghost
Copy link

ghost commented Dec 15, 2016

@dgt41 I'm not sure how it would be implemented exactly for the best results, but my feeling is a media query / CSS solution would work best.

@C-Lodder apologies, thanks for clarifying. I understand. I'm not the decision maker on this matter or any matter relating to production, that's PLT. All I can do is provide input and ask them to take a look. I agree this is something that needs to be decided, but it's not a decision I have the power to make. Even if I could, I wouldn't know where to start with the coding. I can't do this one alone.

@dgrammatiko
Copy link
Contributor Author

but my feeling is a media query / CSS solution would work best.

There's no code for such implementation ( @C-Lodder is following another path for J4). Maybe it's better if you'll do a PR to showcase this...

@ghost
Copy link

ghost commented Dec 15, 2016

Like I said, I wouldn't know where to start with the code. I'll leave the implementation up to the developers, the idea came from the work I've seen in J4. My feeling is that could work here as well.

I'm getting the feeling people believe I'm in charge of more than I really am. PLT makes production decisions, UX is under PLT. In this case, the 3.7 release team has the final say. Not me or the UX team.

I'm happy to help where I can, but putting this all on me isn't going to get us anywhere fast. We can do more if we work together, and we'll have to work together to solve this issue. I can't solve it alone.

@dgrammatiko
Copy link
Contributor Author

Anyways, since this PR was rejected from the UX, I'll close it.

@dgrammatiko dgrammatiko deleted the +++++rfc_toolbar branch December 15, 2016 16:16
@wilsonge
Copy link
Contributor

@cpfeifer Can you please draw some mocks for this

@PhilETaylor

This comment was marked as abuse.

@PhilETaylor

This comment was marked as abuse.

@ghost
Copy link

ghost commented Dec 16, 2016

Thank you for your feedback. This is a volunteer community, we all do this in our spare time and no one team controls everything. I do as much as I can, probably more than I should, and I've had a rough few months so I'd appreciate it if we could keep friendly. Thank you.

@wilsonge Some rough mockups here: #12789 (comment)

@infograf768
Copy link
Member

Hmm... Is the "issue" so important that it needs all this waste of time and efforts? One can live very well with a toolbar getting in 2 lines when not possible otherwise.
But this is only my opinion of course. No need for accusations in any case.

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.

7 participants