search

dynamo-media / redmine_issue_detailed_tabs_time

30 stars 24 forks source link

Easier way to determine the selected tab ad JS restructured #25

Closed CodeInsider closed 10 years ago

CodeInsider commented 10 years ago

Hi,

as already commented on the other pull request, here comes my latest changes.

  1. Just a small comment fix (typo)
  2. I restructured the validTabs in combination with your elsif construct. I think this is more maintainable. I let find just return the first matching tab. The order in which find checks those tabs is defined by the order of the validTabs definition.
  3. I removed some unused functions. On the other hand I changed the way you trigger the click event. $('.tab-history.selected').not('#tab-history_all')[0] && $('.tab-history.selected').not('#tab-history_all')[0].click(); This line just searches for all selected tabs, remove history_all and search for the first indice. If there is only history_all selected, this will return undefined, which prevents the second call to be executed. If the first construct gives some value (for example: tabtime_time), the second construct will be triggered and fire the click()-event.

If you have some questions or concerns, just comment back. Some feedback would be great as usual.

By the way, this should work out of the box, this time my VM works, which means I could directly upload my code to you.

Best regards, Daniel

markedagain commented 10 years ago

im very happy somebody with ruby skills is cleaning up the code. i knew there was better way then my else if but did not want to waist to much time. unless u beat me to it ;), im thinking of creating a helper file to offload some of the logic to it and keep the view file as slim as possible. will prob get around to it in a couple weekends.

CodeInsider commented 10 years ago

Yes this would be good. Many functionalities could be moved. This will help to get a cleaner view. :-)

Good to see, that this plugin is maintained. ;-)

markedagain commented 10 years ago

wish we can track how many ppl clone the repo, im only motivated to keep it maintained when i see ppl actively using it :)

CodeInsider commented 10 years ago

Yes this is too sad. But maybe I can give this a good solution. I'm working on a private project, maybe I'm going to release it in a month or so. Mainly for Redmine purposes.