iMattPro / abbc3

Advanced BBCode Box - An extension for phpBB
https://www.phpbb.com/customise/db/extension/advanced_bbcode_box/
GNU General Public License v2.0
26 stars 28 forks source link

New Look & Feel and some improvements #31

Closed phc369 closed 7 years ago

phc369 commented 8 years ago

Hi, I have made some improvements to the extent that I hope will be to your liking. Please tell me if you want to change what you want.

iMattPro commented 8 years ago

Honestly, this should be broken up into several separate PRs...One for each item in your list. It's too much change in 1 PR.

phc369 commented 8 years ago

Oki. I'll see how I'll do to separate them, since most things are interrelated. have you tried it? Where do you prefer to start? Which item first? Regards Pablo

iMattPro commented 8 years ago

There can be a PR for the new look, a PR for the new undo/redo/copy/paste JS changes, a PR for new BBCodes, a PR for the changes to BBVideo.

No I haven't tried it because the tests fail, which means it won't work.

phc369 commented 8 years ago

Ok... I think there is. Only the first stage regarding the new look.

What do you mean with the testings fail? Sorry, I did not handle very well with Github (for now)

I think the changes to the new Look are very large, it may be why not pass the test?

phc369 commented 8 years ago

It could be that there is some error in the files that perform the testing? I'm asking because I just left as the original the English language file abbc3/language/en/abbc3.php and not pass the test.

iMattPro commented 8 years ago

There are several problems.

  1. First, you should be working on a new branch, not the master branch. https://www.atlassian.com/git/tutorials/comparing-workflows/feature-branch-workflow
  2. I see you are adding font awesome. This is also bad. phpBB 3.2 already has Font Awesome built in. ABBC3 works already in phpBB 3.1 and 3.2, so it can not include font awesome twice. However, relying on Font Awesome breaks backwards compatibility with phpBB 3.1.
  3. The tests are failing because if you look at them, they show that you deleted the ID tag #abbc3_buttons so when you changed that you needed to make sure you replaced #abbc3_buttons everywhere it appears, including in the tests.
  4. You're using "name" inside of span tags which is invalid. Consider using data-name instead. Also use jQuery code instead of javascript for cleaner and more concise code.
  5. There are lots of console.log() which need to be removed.
  6. Also, it simply doesn't display, caused by line 516 of posting_buttons... The longest if statement ever which causes a twig/php fatal error. This should just be a not in set test http://twig.sensiolabs.org/doc/templates.html#containment-operator

Some other problems just in general are that this removes many of the core features of ABBC3 including the ability to sort BBCodes in a custom order, and to easily swap out the icons with new ones or add new ones to custom BBCodes.

While I appreciate all the effort, I think the "new" look is not something I would add to ABBC3, as the look could be accomplished by swapping out new icons and an invisible background bar.

What I would instead be interested in from your work is:

iMattPro commented 8 years ago

FYI: It also may be possible to instead make your work into an "Add-On" for ABBC3, wherein it is a stand-alone extension that you can build and maintain yourself, that will replace/overrule ABBC3's templates with your new ones (or use ABBC3's events to inject your own code changes), much like the way ABBC3 overrules phpBB's posting buttons.

phc369 commented 8 years ago

Thank you so much Matt for all as detailed explanation, with all this information I will see how to correct all the mistakes I've made. Thank you, this has been a great help.

I'll get to correct these errors and see how to adapt the features you are interested in a new request.

Now I am finishing a new feature that make the bbcode buttons act like switches when we have a selected text. (As the quick-reply of comments works on github) That is, if we have a selected text and the outer or inner edges have the same bbcode when pressed again, this is removed instead of refitting. Although a little more intelligent, as it also works if the selected text is selected bbcode incompletely (I hope you can understand what I mean by my bad English)

So, the first would make to do is delete this branch and make a "Fork" of "develop-3.2.x Branch" ?

I'm attaching the extension with this new feature so that you give a look, although it is not yet finished.

Thank you for all Matt, I'll get to recreate the same without the new Look.

Pablo

2016-05-23 20:14 GMT-03:00 Matt Friedman notifications@github.com:

FYI: It also may be possible to instead make your work into an "Add-On" for ABBC3, wherein it is a stand-alone extension that will replace ABBC3's templates with your new ones (or use ABBC3's events).

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/VSEphpbb/abbc3/pull/31#issuecomment-221123272

phc369 commented 8 years ago

I forgot to attach the file. abbc3_3.2.x.zip

iMattPro commented 8 years ago

You should make a new branch with a new name based off the master branch. The develop-32x branch is for something else, and really only for work that would be incompatible with phpBB 3.1.

phc369 commented 8 years ago

Thanks

phc369 commented 8 years ago

Ok, I think now is more enjoyable.

Now it works with icons in the folder /images/icons/ (although they are in .png) instead of the FontAwesome. And the feature to sort bbcode is also maintained.

There is the possibility to separating the bbcode into different panels and sub-groups.

To do this, just add some bbcode that must meet the following condition: for Panels: must contain the word "end" and "panel" for Sub-Groups: must contain the word "end" and "group" the words must be separated by the symbol -

for example:

--end-panel-1-- (to indicate that there ends the panel 1) -end-p1-group-1- (to indicate that there ends the group 1 of the panel 1) -end-p1-group-2- (to indicate that there ends the group 2 of the panel 1)

I attach the extension because now the test fails by .png files and the new migration file.(where new bbcode, the panels and groups and the predetermined order defined)

Regards, Pablo abbc3-beta.zip