dynamo-media / redmine_issue_detailed_tabs_time

30 stars 24 forks source link

add selected tab to brower url #23

Closed CodeInsider closed 10 years ago

CodeInsider commented 10 years ago

This should solve issue #21.

I used the same mechanic as redmine itself does it.

markedagain commented 10 years ago

oh, looks like we both worked on it on same time,

markedagain commented 10 years ago

im a bit confused as to what the change is. i get the fallback one, just in case someone is trying to load a invalid tab name to url. but the first change where u alter the js, and do a replace of the hash tag to a param i get a bit confused. let me know if im miss understanding it. with my little changes, we should now be able to use either or. but the hash tag ends up getting precedence at the end.

markedagain commented 10 years ago

merged manually, u also had forgotten a '+' in js ;) i got what u are doing, i did not realize i can change url without redirecting, i left hash tag codes in there just in case browser does not support it. hmm, now that i think about it, maybe i should of moved it into a if !statment checking if browser does not support it and do hash tag stuff. ill do on next commit, are u working on other features, i was thinking of doing the permissions of the tabs per role next

CodeInsider commented 10 years ago

Hi sorry for the +.

Yes I started my work for some minutes before i send you the commit. I wasn't sure if your working on it at the moment.

I normally programm in my VM for Ruby, but this time the guest addition is a bit broken, which leads me to retype everything. Not so a great work. :-D

Yes i just manipulate the browser history, if it is possible. I'm not he friend of reloading the page and give traffic to the server, just for some neat url change. ;-)

The fallback thing is just in case someone misstyped. Your plugin will show up regularly with history_all selected, but the tab won't display as selected. The fallback just give him the focus.

No I'm not working on something else in this plugin at this time.

markedagain commented 10 years ago

i agree with you , i also dont want to add any extra page reloads that dont need it. thank you for the work

CodeInsider commented 10 years ago

Hi,

maybe you can add the validTabs? This will give a better display of the tabs, even if the user mistyped in the url manually.

markedagain commented 10 years ago

hmm, so turns out i did not push your changes, got does ready for next commit, but i noticed we both made a mistake. By changing the default tab to comments, we are not changing the actuall the comment to visible and the rest of the divs to display none. im going to see if i can pull this off by rearanging the code a bit, or worst case, do some extra work in js on page load

CodeInsider commented 10 years ago

Hm I'm not quite sure if I get your problem. Maybe just too late. :smiley:

Can you give me a hint, how to check the problem to see what's the mistake behind?

markedagain commented 10 years ago

i still have not pushed your code out yet, as i found another typo and im now working on a few fixes, but when the page loads with your code and the comment tab visual looks like its selected, the rest of the actual entries are all visible

CodeInsider commented 10 years ago

Ah thanks for this. Now I get it. :-) Maybe I can change this.

Maybe you can give me some quick feedback if you still need my repo after the latest pull request. If not, I would like to truncate it, just for a better overview for myself. I will fork a new repo if I try to change something or if I'll translate new strings. :-)

markedagain commented 10 years ago

Hey @CodeInsider the changes i ended up fixing using js. not the best solution but it seems to work. i wont be working on this today. so if u want to make any changes ill be more then happy to pull them in

CodeInsider commented 10 years ago

Hi markedagain,

okay thanks for the info. I change some little parts which could be a little bit easier. I will sent you a pull request at 6.