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.

Sign up for GitHub

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

Jump to bottom

Improved table layout #20

Closed
wants to merge 1 commit into from
Closed

Improved table layout #20

wants to merge 1 commit into from

Conversation

eisuke
Copy link
Member

@eisuke eisuke commented Nov 15, 2016

  • Show tags before description.
  • Remove "Details" buttons and clickable table row.

Dashboard

before after
before_dashboard after_dashboard

All Job Definitions

before after
before_all_job after_all_job

cc: @cookpad/dev-infra

@hogelog
Copy link
Contributor

hogelog commented Nov 16, 2016

Kuroko 2 displays tag as badge on other view.
image

Why this PR using [foo] [bar] plain text?

@eisuke
Copy link
Member Author

eisuke commented Nov 16, 2016

@hogelog

Why this PR using [foo] [bar] plain text?

The badge or label class express what you can click, Square brackets express what tags are just text.
Is it confusing?

eagletmt
eagletmt reviewed Nov 16, 2016
View reviewed changes
app/assets/javascripts/kuroko2/narrow_job_definitions.js
@@ -1,9 +1,17 @@
jQuery(function ($) {
var link_to_table_row = function(){
$('tr[data-href]').css("cursor", "pointer").click(function(e){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this kind of pseudo-links.
It cannot handle non-standard clicks such as opening context menu (right-click) and opening the link in new tab (middle-click).

@hogelog
Copy link
Contributor

hogelog commented Nov 16, 2016

It seems that current Kuroko2 doesn't treat badge or label as a clickable element.

image
image
Above user group status, job status and other some badge element isn't clickacle.

So I feel naturally to treat tag as badge.

But above opinion is based on my personal feeling.

I will not stop if you want to.

@k0kubun
Copy link
Contributor

k0kubun commented Nov 16, 2016

The badge or label class express what you can click

How about linking the badge to https://kuroko2.ckpd.co/?tag[]=foo?

@eisuke
Copy link
Member Author

eisuke commented Nov 16, 2016

@hogelog

Above user group status, job status and other some badge element isn't clickacle.

Good point. I didn't consider other label classes.
Also @eagletmt 's #20 (comment) is good point too.

I begin to feel that this PR is not good improvement 😢
I retry to make the pull request supporting tags display only.

@eisuke eisuke closed this Nov 16, 2016
@eisuke eisuke deleted the design branch November 16, 2016 04:44
@eisuke eisuke mentioned this pull request Nov 16, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eagletmt eagletmt eagletmt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eisuke @hogelog @k0kubun @eagletmt