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

WINDUP-1254 #153

Merged
merged 5 commits into from Jan 10, 2017
Merged

WINDUP-1254 #153

merged 5 commits into from Jan 10, 2017

Conversation

mareknovotny
Copy link
Contributor

Copy link
Collaborator

@klinki klinki left a comment

Choose a reason for hiding this comment

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

Code looks good, I have just 2 comments:

  • breadcrumbs are not displayed on active executions page
  • I think visibility of the text didn't improve much. Maybe increasing font size could help? Or bold text. Or darker color :) That one is probably the best.

@jsight
Copy link
Member

jsight commented Jan 9, 2017

@mareknovotny - +1 to both of David's comments. I do like the idea, though. It looks nice locally, but would probably be better with a slightly larger font size.

@mareknovotny
Copy link
Contributor Author

Oops, I broke the Executions list layout then with the last css change. I will look at it.

With the next suggestion to font size - I can agree with that even I was focused on positioning and color background changes for WINDUP-1254. Anyway I will try to raise the size to fit the rest of page(s).

@mareknovotny
Copy link
Contributor Author

hmm, after hours of discovery why it is not there, i haven't found anything in styles, but noticed that the anchor is generated without text, so it is rendered, but physically the breadcrumbs link is empty.

Fixes issue with breadcrumbs not visible on global executions list.
@klinki
Copy link
Collaborator

klinki commented Jan 10, 2017

It was actually my fault. I forgot to add displayName into global executions list route.

@mareknovotny
Copy link
Contributor Author

it looks now as this
screenshot from 2017-01-10 14-19-36

@mareknovotny mareknovotny changed the title Windup 1254 WINDUP-1254 Jan 10, 2017
@jsight jsight merged commit 79f1011 into windup:master Jan 10, 2017
@mareknovotny mareknovotny deleted the WINDUP-1254 branch January 16, 2017 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants