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

Propose a new fresh UI - Tabs! #135

Closed
herrvigg opened this issue Jun 22, 2018 · 17 comments
Closed

Propose a new fresh UI - Tabs! #135

herrvigg opened this issue Jun 22, 2018 · 17 comments
Labels
enhancement New feature or request legacy PR Legacy PR imported from original repo

Comments

@herrvigg
Copy link
Collaborator

Issue by pedro-mendonca
Thursday Apr 16, 2015 at 21:38 GMT
Originally opened as qTranslate-Team/qtranslate-x#135


The Show/Hide sections isn't very funcional.
It needs a lot of scrolling and a litte ugly.

I've maintained the whoole code, except section navigation.
It uses WordPress native css for Tabs.

Some old organization might need a cleanup, but for now I've maintained everything as is to minimize commit diffs.
I've pushed partial commits to make your work easyer to understand what changed, as the whole diff is too long to understand.

The code was adapted from a living example: WP-Maintenance-Mode tabbed admin settings

There are some issues not solved yet, but I believe you @johnclause are the one who can solve it:

After saving changes it goes back to first tab.
The original code keeps in same tab after saving, but in this case <form action="<?php echo $clean_uri;?>" method="post"> is the problem. If <form> have action="" it works like a charm.
I didn't touch this because I think it's better in your hands from this point beyond.

Please check this out, it really looks way better and works great!
:)


pedro-mendonca included the following code: https://github.com/qTranslate-Team/qtranslate-x/pull/135/commits

@herrvigg herrvigg added enhancement New feature or request legacy issue Legacy issue imported from original repo labels Jun 22, 2018
@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Friday Apr 17, 2015 at 14:58 GMT


Hi Pedro, I will play with this after 3.3 release. This seems to be a pretty big change, which I also planned to do one day, I really appreciate that you took the initiative. I thought I would have to change functions qtranxf_admin_section_start/end only for that, but it seems to have taken many more changes. I will review it in a few days. Thanks!

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Friday Apr 17, 2015 at 15:09 GMT


Yes, of course.
There are two options, the tabs reloading the page and the 5 forms being separate, or the tabs are just hidden and shown without reloading.
I don't really know what will you prefer, I just did a change in the shown/hidden sections using the tabs, I did no change at all in the overall structure.
To make it easier, I've made the partial commits with less changes each one.
I took out the qtransf_admin_section_start/end the show/hide beaviour and used basic jquery for the nav tabs. Then I've added the necessary divs wrapping those admin_sections. i did it INSIDE the <form> because it's just one for sections1, 2 and 3.
A lot of changes occur just because of the conde indenting, I can revert that if it helps to understand the commit diffs.
If you want I can try to improve it by puting inside those ..._start/end functions the most part of the wrapping code.

@herrvigg
Copy link
Collaborator Author

Comment by qtranslateteam
Friday Apr 17, 2015 at 15:25 GMT


I did not follow through your code yet, and I am still not sure why changes
other than inside _start/end functions (including their arguments) are
needed. I thought I would keep it encapsulated by sections as it is now,
and only re-implement _start/end to show tabs. Is that possible? And I also
thought that I will not ask people to reload page to change a tab, your
second option, I guess, this is how you did it too?

On Fri, Apr 17, 2015 at 8:09 AM, Pedro Mendonça notifications@github.com
wrote:

Yes, of course.
There are two options, the tabs reloading the page and the 5 forms are
different, or the tabs are just hidden and shown without reloading.
I don't really know what will you prefer, I just did a change in the
shown/hidden sections using the tabs, I did no change at all in the overall
structure.
To make it easier, I've made the partial commits with less changes each
one.
I took out the qtransf_admin_section_start/end the show/hide beaviour and
used basic jquery for the nav tabs. Then I've added the necessary divs
wrapping those admin_sections. i did it INSIDE the

because it's
just one for sections1, 2 and 3.
If you want I can try to improve it by puting inside those ..._start/end
functions the most part of the wrapping code.


Reply to this email directly or view it on GitHub
qTranslate-Team/qtranslate-x#135 (comment)
.

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Friday Apr 17, 2015 at 15:31 GMT


Yes, as I said, the basic structure is the same, so, no reloading.
After looking at it again now that my work was completed, I think I can move a lot of the section wrapping to _start/end functions. I'll do this and remove the indenting for now, it was necessary for me to understand the code but now I can revert it.

I'll do some changes and add to the current commits, ok?
Meanwhile, I'll keep pushing master commits to this branch so that it won't get to far behind and at any moment it can be included in master. Because of this some more merging commits will keep apearing and the list will increase, but in the end, the commits diffs will reduce.

@herrvigg
Copy link
Collaborator Author

Comment by qtranslateteam
Friday Apr 17, 2015 at 15:35 GMT


indenting

I use tab size equal to 2. Most editors have an option for tab size, which
helps a lot. Is that what you mean?

On Fri, Apr 17, 2015 at 8:31 AM, Pedro Mendonça notifications@github.com
wrote:

Yes, as I said, the basic structure is the same, so, no reloading.
Looking at it again after the work is done, I think I can move a lot of
the section wrapping to _start/end functions. I'll do this and remove the
indenting for now, it was necessary for me to understand the code but now I
can revert it.

I'll do some changes and add to the current commits, ok?
Meanwhile, I'll keep pushing master commits to this branch so that it
won't get to far behind and at any moment it can be included in master.
Because of this some more merging commits will keep apearing and the list
will increase, but in the end, the commits diffs will reduce.


Reply to this email directly or view it on GitHub
qTranslate-Team/qtranslate-x#135 (comment)
.

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Friday Apr 17, 2015 at 15:38 GMT


It could be easier for you to make a new fork and then to copy relevant pieces from your local copy into the new fork?

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Saturday Apr 18, 2015 at 14:58 GMT


@johnclause
I just pulled some more commits to this.
I've kept myself within the same branch, locally I've started all over with the current master files, changed only the necessary code to switch from Hide/Show to Tabs and commited the file to ui-tabs branch.
This shows a lot of reversing changes in the particular commit pedro-mendonca/qtranslate-x@cd6a468 because it's on top of previous changes, but as I did it from scratch without the overall code cleaning, the global commit diffs that appear here are the ones I've really made, and are just the simple changes needed to switch GUI to tabs.

Summary:
Included admin container classes in _start/end
included global that contains the admin sections. There are two of them because you have also two wraps.
Included navigation tabs and necessary jquery script
Used variables to define section titles, to avoid redundancy in reusing the titles for tabs and alt text.

Please check if it's simpler now.

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Saturday Apr 18, 2015 at 15:03 GMT


After this, I think some stuff can be deleted:

  • The function that used to Show/Hide
  • The var $section from the function _start is no longer needed as the Section Title is defined in the Navigation Tabs
  • The cookie request and function to remember which sections where shown/hidden

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Thursday Apr 30, 2015 at 03:21 GMT


Pedro, I am sorry for the delay, I will play with this after 3.3 is out. Otherwise there will be not much new in 3.4 ;)

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Thursday Apr 30, 2015 at 06:24 GMT


That's ok :)
Do you want me to merge the full recent stuff from master in ui-tabs branch to keep it up to date?

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Thursday Apr 30, 2015 at 17:29 GMT


I do not know, it still says that it can be merged automatically. I did not change the file affected much and will probably not change before merging this PR.

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Thursday Apr 30, 2015 at 17:55 GMT


Maybe it's because ui-tabs branch isn't on top of latest commit on your master.
I'm doing an update, it will increase the list of commits above but will be updated on top of current master.

@herrvigg
Copy link
Collaborator Author

Comment by johnclause
Tuesday May 05, 2015 at 15:33 GMT


oops ... Now it says it cannot merge? What happened?

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Tuesday May 05, 2015 at 15:34 GMT


It got out of sync because of last updates, give-me a minute to sync it.

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Tuesday May 05, 2015 at 15:47 GMT


Ok, I see that there are recent changes in the same files, let me rearrange that and I'll notice you.

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Tuesday May 05, 2015 at 23:23 GMT


This PR is getting very confuse with syncing between 3 branches.
I'm closing this and making a brand new clean PR.

@herrvigg
Copy link
Collaborator Author

Comment by pedro-mendonca
Tuesday May 05, 2015 at 23:42 GMT


I've rewritten the code from scratch in a brand new ui-tabs branch and pulled it again in qTranslate-Team/qtranslate-x#153

@herrvigg herrvigg added legacy PR Legacy PR imported from original repo and removed legacy issue Legacy issue imported from original repo labels Jun 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request legacy PR Legacy PR imported from original repo
Projects
None yet
Development

No branches or pull requests

1 participant