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

[3.x] Ability to view job details in recent jobs overview #751

Merged
merged 6 commits into from
Feb 4, 2020
Merged

[3.x] Ability to view job details in recent jobs overview #751

merged 6 commits into from
Feb 4, 2020

Conversation

Cannonb4ll
Copy link
Contributor

@Cannonb4ll Cannonb4ll commented Jan 31, 2020

I sometimes find myself in a situation where I need to know what data is passed into the job (whatever its status was).

This is similar to reading failed job data in the Failed Jobs tab.

Img
Img

@themsaid themsaid self-requested a review February 1, 2020 10:34
@driesvints driesvints changed the title Ability to view job details in recent jobs overview [3.x] Ability to view job details in recent jobs overview Feb 3, 2020
@driesvints
Copy link
Member

driesvints commented Feb 3, 2020

@Cannonb4ll this looks really good 👍

I was thinking that maybe we can also tackle #388 together with this? Maybe in a new "Tags" card below the "Data" card? How does the "Tags" data now look in the first panel with lots and lots of tags as displayed in the issue?

@Cannonb4ll
Copy link
Contributor Author

@driesvints Oh dear, I have not checked how it would look with a lot of tags. I will check this, please wait before merging as I could do some nice stuff with this 👏

Will send new updates in this PR.

@Cannonb4ll
Copy link
Contributor Author

@driesvints What do you think of this?

The overview shows a max of 3 tags, and then if there's more than 3 it will show (50 more). The job detail view (which I created earlier) will then also show the tags via vue-pretty-print at the bottom.

I have also added a collapse feature (default from bootstrap, does not come with any additional libs) to make it more easier to quickly read the tags card if you have a lot of data.

See screenshots below for reference, let me know what you think!

img
img2

@themsaid
Copy link
Member

themsaid commented Feb 4, 2020

There will always be people who want to see all tags in the table even if the layout isn't the best, specially that one tag name can already break the layout if it's really long App\Forge\Servers\Networking\FirewallRules\Rule:12341.

I don't think we can really go forward with trimming without seeing other issues being opened asking for going back to showing all tags.

@driesvints
Copy link
Member

@Cannonb4ll I've talked to @themsaid about this and it's best that we do the tags box in a different PR. Can you revert your last commit and send in a separate PR for it once this one is merged? Thanks!

@Cannonb4ll
Copy link
Contributor Author

Reverted.

@driesvints
Copy link
Member

Thanks!

@taylorotwell taylorotwell merged commit 46c8070 into laravel:3.0 Feb 4, 2020
@driesvints
Copy link
Member

@Cannonb4ll feel free to send in a follow up pr!

@Cannonb4ll
Copy link
Contributor Author

@driesvints I would, but then probably only for the tags on the job detail, not the index I think? As @themsaid did not agree on limiting results on the index.

@driesvints
Copy link
Member

@Cannonb4ll feel free to send in pr with the changes to the index you made.

@brunogaspar
Copy link

@Cannonb4ll Noticed a small issue, if the job hasn't run or is running, the completed at shows 1970-01-01 01:00:00 :)

I can try to send a pr to address the issue tomorrow or so, just letting you know in case you want to fix it, not a big deal for now hehe

@Cannonb4ll
Copy link
Contributor Author

No need, thats fixed in #755

If that wont be merged ill send it in loose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants