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

Change Nr. is a link to review detail #305

Merged
merged 2 commits into from Jan 16, 2017

Conversation

Projects
None yet
3 participants
@jbesta
Copy link

commented Jan 12, 2017

On search results page, user may open list of parameters about a change. One of this parameters is GERRIT_CHANGE_URL which is a URL to Gerrit review server and user may click on (link) to navigate to Gerrit review detail. This change makes the same link from Change Nr so that user may navigate to Gerrit review detail directly.

@scoheb

This comment has been minimized.

Copy link
Contributor

commented Jan 12, 2017

Looks good. What about a test?

Something like GerritTriggerProjectHudsonTest#testPopulateDropDown where you verify that the HtmlElement exists?

@@ -132,8 +132,19 @@
id="check${theId}"
onClick="rowSelected('${theId}')"/>
</td>
<td onClick="activateRow('${theId}')">

This comment has been minimized.

Copy link
@rsandell

rsandell Jan 12, 2017

Member

I feel the number is the most common place you'd want to click for activating the row, so why remove it??

This comment has been minimized.

Copy link
@jbesta

jbesta Jan 12, 2017

Author

Initially, I removed it, because it activated row even if you clicked on the link to navigate to Gerrit.
I added the functionality back in following commit and now click event is consumed after clicking on the link, so it no longer happens.

Jindrich Besta added some commits Jan 4, 2017

Jindrich Besta
Link to change detail page from search results
Change-Id: I9bd8b5fa7307a9408dedb181fda3790104652f8c
Jindrich Besta
Add back `activateRow` functionality
was removed in 2e0b378

Change-Id: I851256a8061ff30b8c6bfefa4c2d6e1fa6824485

@jbesta jbesta force-pushed the jbesta:feature/change_nr_link branch from 87b98d4 to 5e57797 Jan 12, 2017

@rsandell rsandell merged commit 7d8756f into jenkinsci:master Jan 16, 2017

1 check was pending

Jenkins Jenkins is validating pull request ...
Details

@jbesta jbesta deleted the jbesta:feature/change_nr_link branch Jan 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.