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
Feature: Starred groups with option to restrict view to these projects only #74
Conversation
… projects on the Repositories page, and a UI toggle to set this property.
…o, and preserves the PageParameters.
…anels to output more IDs and CSS classes for use in testing.
This looks like a very nice addition. The only thing I am not crazy about is persisting the ui config in the user service. I think that show starred/show all would better be handled by a new Gitblit settings cookie. What do you think of that? |
As you may use different computers at the same time (like I do - most times I use two in parallel, sometimes three) I think it would be better to have the settings persisted on server side. Otherwise I have to select my favorites three times. Furthermore, developers tend to clear their caches from time to time and if they develop a web app, some of them might need to clear the cached cookies as well and they probably will feel bothered selecting which cookies to delete and which cookies to keep each time. From the perspective of usability for the gitblit user I think it's better to persist the settings on server side. Other opinions? |
I've reviewed this more thoroughly and here are my thoughts.
|
Hi James, Alice and me just discussed your feedback. we would like to divide the changes to make into two packages. So, we concentrate on the core functionality first, then we merge back to the master, before we extend the UI again. There is one question remaining regarding 7.: What should we use instead of the Translation class then? Can you give us an example or point us to a line in the existing code, which does it the right way? We think that we can finish 1., 4., 5., and 6. (and probably 7. if we know how to do it correctly) until end of next week. |
That sounds fine. I'm having a hardtime finding an example in my code. :( You can look at NavigationPanel to see the use of getString(key). But you should also notice that this method is used inside a repeater. Wicket complains obnoxiously if you call getString from the constructor of a panel. In your implementation you would have to do that or pass a Localizer instance from the owning page as a constructor argument. One alternative is to specify the translation key in your markup using the wicket:message notation but to do that you would have to create two buttons and show/hide buttons. Another alternative would be a dropdown list: I would favor a dropdown here because there may be reasons to add more selections in the future; a toggle button does not scale. Gitblit currently has Bootstrap 2.0.4 (I think) so the docs may be out of sync. One thing I try to keep tight control of is Wicket page sessions. When you use Wicket links or AJAX you create a serialized version of the page in a user page-session-store. For some pages this is absolute must (Edit Repository) for most, though, I try hard to avoid that and to keep pages session-less. This is not the same as the servlet container session. A page with Wicket state will lead to link failures if Wicket thinks your page has expired. One way I work around that is by minimizing use of Wicket-generated links by passing explicit urls with parameters REST-ish or by storing important state in the user session. It may not be a problem here, but it is something to keep in mind. A good example of this is the repositories page for an Admin or Owner and the delete repo links. Those will fail after a session timeout because they use Wicket urls, Javascript, and rely on the a page session store. In contrast, the edit links should always work because they are manually generated and point to REST-ish urls. I'm not in love with Wicket, but it is what we've got. As for the regex, I don't think you need anything different UI-wise to support regex. It should be straight-forward and internal. I also now notice this line: You shouldn't need that. Check out RepositoryModel.projectPath. |
Hi Alice & Sarah, I have implemented a slightly different starring feature - which at the moment does not touch the repositories page, but likely should. Thanks for contributing your ideas and participating in a discussion with me, even though I ultimately decided to implement this slightly differently. Good luck with your scm-manager transition! -J |
Added a feature for when the application has the "grouped" setting for repositories: users can select particular projects/groups that they are interested in (similar to starring a repository on GitHub) and choose to view only those repositories on the Repositories and Activity pages. Filter parameters are preserved when making changes. The user view preference is saved as a property on the user object and is therefore "sticky".
In code we have gone with the more neutral terms "selected" and "unselected", but in the UI have opted for "starred/unstarred". The current English labels for the view preference toggle are therefore
show all repositories
andshow starred projects only
while those for the buttons to select/unselect a project are:☆ star
(unicode white star) and★ unstar
(unicode black star). These can of course be customized according to preference.A Selenium test is provided.