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

Feature: Ability to hide header and footer #57

Closed
wants to merge 2 commits into from
Closed

Feature: Ability to hide header and footer #57

wants to merge 2 commits into from

Conversation

drekbour
Copy link
Contributor

@drekbour drekbour commented Apr 8, 2014

Cookie-based checkbox added to settings panel which hides the header and footer
(now rebased onto master to include similar work on a colour-blind checkbox)
Closes #38

@drekbour
Copy link
Contributor Author

@jan-molak, I keep requiring to rebase and patch up to keep this current. Is there a problem with integrating it?

@jan-molak
Copy link
Member

Hey @drekbour, sorry for not giving you my feedback earlier;
The footer contains a <notifier /> directive, so if you hide the footer and Jenkins goes down, people won't see the error message that would normally be displayed within that area.
Perhaps if we had an alternative method of notifying the user we could avoid introducing this regression? Something like growl might be suitable I think.

@drekbour
Copy link
Contributor Author

As js is not my forte, I'd rather leave toaster pop ups to those who could do so easily. Fair enough on the reduction in function though - I expect it's easy to make the footer visibility also conditional on the error being activated though...

@drekbour
Copy link
Contributor Author

Resolved.

@drekbour
Copy link
Contributor Author

@jan-molak bump. I'm trying to clear my dev backlog for the year so let me know if there is anything further to do here!

<input data-ng-model="settings.showHeader"
data-ng-false-value="0"
data-ng-true-value="1"
id="settings-colour-blind" type="checkbox" />
Copy link
Contributor

Choose a reason for hiding this comment

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

This 'id' is a copy/paste error and should be 'settings-show-header'.

@Brantone
Copy link

For the <notifier /> directive issue, I wonder if that could be placed outsider the footer and hidden until it's needed?

@drekbour
Copy link
Contributor Author

drekbour commented Jun 3, 2015

@jan-molak, I keep requiring to rebase and patch up to keep this current. Is there a problem with integrating it? It's a year old now!
I've corrected the Notifier issue...

@drekbour
Copy link
Contributor Author

@jan-molak, I've rebased this again. Can you please review and integrate (or say why you don't like the feature) - it's been ~20 months now.

@jan-molak
Copy link
Member

Hi @drekbour and apologies for not getting back to you sooner.

I think that the underlying problem here is that Build Monitor didn't cope very well with a large number (hundreds) of projects displayed on one screen - as depicted on the screenshot attached to #124.
That's because I've never anticipated people using it to monitor so many things! 😄

Having said that, many thanks for your pull request. It served as a valuable reminder that the problem needed to be addressed. It also highlighted that the UI required a bit of a rework to make it more flexible and capable of supporting busy Jenkins installations, such as yours.

I've incorporated suggestions from yourself, @saan800 (#124) and @davidparsson (#165) and came up with a number of changes to the UI that should hopefully address the points you guys have raised.

If you have a moment, I'd be grateful if you could download the latest build, try it out and let me know your thoughts!

All the best,
Jan

@drekbour
Copy link
Contributor Author

I tried the latest build and it's fine but I can't comment on the efficacy for large volumes as our usecase is not "200 jobs". We resolved that one years ago by allowing the use of filters from the ListView so it can monitor failed/building jobs only. We have many more usecases speaking to why screen real-estate matters:

  1. The master build monitor is on a small screen at the end of long desk (politics...). It may only have a few items on but I don't want nn% of the vertical space taken by static content making me squint harder.
  2. Users often open a local build monitor in a window on their desktop (with perhaps a more personalised set of jobs)
  3. We may yet add this as a kind of "iframe/portlet" in a much broader-scoped dashboard
  4. We want information that is relevant to our dev process. Green is good; Red is bad; Title, logo, plugin version, homepage and author are noise.

I worry point 4 (the only one that really matters) is clashing with some other concern?

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.

Possibility to hide board title
4 participants