Skip to content

tinymce - toolbar tidyup#7593

Closed
brianteeman wants to merge 6 commits intojoomla:stagingfrom
brianteeman:tinymce
Closed

tinymce - toolbar tidyup#7593
brianteeman wants to merge 6 commits intojoomla:stagingfrom
brianteeman:tinymce

Conversation

@brianteeman
Copy link
Copy Markdown
Contributor

Hopefully the screenshots show the proposed changes of this PR

Advanced Mode - Default

Before

advanced-mode-old

After

advanced-mode-new

rvlq

Extended Mode (enable in the plugin)

Before

extended-mode-old

After

extended-mode-new

extended-mode-new - small

[Note this cant be tested with #7170 as it needs a small update to match the changes there]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

May I suggest:

$toolbar1 = implode(' ', $toolbar1_add) . ' | ' . implode(' ', $toolbar2_add) . ' | ' . implode(' ', $toolbar3_add) . ' | ' . implode(' ', $toolbar4_add);

This will add a small space in-between those toolbars, I think is easier for navigation

@brianteeman
Copy link
Copy Markdown
Contributor Author

@DGT41 your last recommendation made Travis sad. I dont know the best way to make Travis happy again

@dgrammatiko
Copy link
Copy Markdown
Contributor

Arrrg, the last edit exceeds the allowed characters per line. But don’t try to do anything I will make a PR here shortly to sort this out

@brianteeman
Copy link
Copy Markdown
Contributor Author

I am not trying to do anything.

@dgrammatiko
Copy link
Copy Markdown
Contributor

@brianteeman Brian check brianteeman#13

@brianteeman
Copy link
Copy Markdown
Contributor Author

@DGT41 merged

@crystalenka
Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 032f4f4


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

@nikosdion
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 032f4f4
It is common sense and looks SO MUCH BETTER in limited width environments (narrow windows, double column edit pages, mobile phones etc). +1 from me.


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

@brianteeman
Copy link
Copy Markdown
Contributor Author

Thanks for testing - setting RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 1, 2015
@zero-24 zero-24 modified the milestones: Joomla! 3.4.5, Joomla! 3.5.0 Oct 1, 2015
@roland-d
Copy link
Copy Markdown
Contributor

@DGT41 This PR conflicts with your other PR #7170 which is merged. Can you check how this relates to your PR and if it is still applicable? Thanks.

@brianteeman
Copy link
Copy Markdown
Contributor Author

where is the conflict - github isnt showing it to me

Unless @DGT41 changed his PR then it should be fine as it was tested with it

@roland-d
Copy link
Copy Markdown
Contributor

@brianteeman The conflict is not shown here because the PR is against staging and I am going to merge it into the 3.5-dev branch. In the 3.5-dev branch another Tiny PR by @DGT41 has already been merged.

The conflict is here:
This PR:
https://github.com/joomla/joomla-cms/pull/7593/files#diff-d93fcfa294cc59e70963805efe9f303bR619

Other PR:
https://github.com/joomla/joomla-cms/pull/7170/files#diff-d93fcfa294cc59e70963805efe9f303bR625

same lines have been changed.

@brianteeman
Copy link
Copy Markdown
Contributor Author

Ah ok - thanks checking against 3.5-dev now the patch is still needed but it does indeed conflict and revert the changes made by @DGT41 in #7170

Rebasing this PR is beyond my skillset (I never get it right) so can someone do a PR against my branch to update it - its a simple update - thanks

@roland-d
Copy link
Copy Markdown
Contributor

@brianteeman a rebase is not needed, if @DGT41 can do a PR against your branch we should be good to go. Thanks.

@brianteeman
Copy link
Copy Markdown
Contributor Author

I meant have used the wrong terminology ;)

On 24 October 2015 at 12:09, RolandD notifications@github.com wrote:

@brianteeman https://github.com/brianteeman a rebase is not needed, if
@DGT41 https://github.com/dgt41 can do a PR against your branch we
should be good to go. Thanks.


Reply to this email directly or view it on GitHub
#7593 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@dgrammatiko
Copy link
Copy Markdown
Contributor

@brianteeman this pr is against staging the conflict comes with #7170 which is merged in 3.5-dev. I think if you rebadge this against 3.5-Dev then the conflict will be revealed

@roland-d
Copy link
Copy Markdown
Contributor

@DGT41 Brian doesn't know how to rebase against 3.5-dev. My question is, can you provide a PR against this branch that solves the conflicts?

@brianteeman
Copy link
Copy Markdown
Contributor Author

Looks like the PR @DGT41 did on ym repo just broke it even more

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Oct 26, 2015

Closing as we have a new PR here #8159

@zero-24 zero-24 closed this Oct 26, 2015
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 26, 2015
@brianteeman brianteeman deleted the tinymce branch October 26, 2015 08:35
roland-d added a commit that referenced this pull request Nov 2, 2015
roland-d added a commit that referenced this pull request Nov 2, 2015
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