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

General Style/Layout enhancements #103

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

General Style/Layout enhancements #103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
4bfa529
CSS: Align `.cache-notice`
flourish86 May 15, 2023
46a51db
EventListItem: Do not render footer
flourish86 May 15, 2023
da0cbf5
CSS: Align EventListItem in controls vertically
flourish86 May 15, 2023
f2fc031
list-item.less: Do not wrap `.meta` element
flourish86 May 16, 2023
c02bed6
event-source-badge: Optimize colors in light mode
flourish86 May 16, 2023
040fd79
CSS: Optimize `.entry .title` spacing
flourish86 May 16, 2023
e9ea20a
Add `.full-width` class to `.content` containers
flourish86 May 17, 2023
c8d21b5
Label create buttons consistently
flourish86 May 17, 2023
308fda7
Clarify button labels in forms
flourish86 May 22, 2023
b96e2bc
Realign buttons in `.form-controls`
flourish86 May 22, 2023
eb65ebd
CSS: Clarify disabled state
flourish86 May 22, 2023
2852a4d
ChannelForm: Clarify delete button label
flourish86 May 22, 2023
e45eb70
ContactForm: Clarify delete button label
flourish86 May 22, 2023
4b33a04
event-rule-details.less: Remove unused `. new-event-rule
flourish86 May 24, 2023
d06d36c
Make channel form button labels consistent
flourish86 May 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions application/controllers/ChannelsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public function indexAction()
$this->addControl($searchBar);
$this->addContent(
(new ButtonLink(
t('Add Channel'),
t('New Channel'),
Url::fromPath('notifications/channels/add'),
'plus'
))->setBaseTarget('_next')
Expand All @@ -101,7 +101,7 @@ public function indexAction()

public function addAction()
{
$this->addTitleTab(t('Add Channel'));
$this->addTitleTab(t('New Channel'));
$form = (new ChannelForm($this->db))
->on(ChannelForm::ON_SUCCESS, function (ChannelForm $form) {
Notification::success(
Expand Down
4 changes: 2 additions & 2 deletions application/controllers/ContactsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function indexAction()
$this->addControl($searchBar);
$this->addContent(
(new ButtonLink(
t('Add Contact'),
t('New Contact'),
'notifications/contacts/add',
'plus'
))->setBaseTarget('_next')
Expand All @@ -109,7 +109,7 @@ public function indexAction()

public function addAction()
{
$this->addTitleTab(t('Add Contact'));
$this->addTitleTab(t('New Contact'));

$form = (new ContactForm($this->db))
->on(ContactForm::ON_SUCCESS, function (ContactForm $form) {
Expand Down
5 changes: 3 additions & 2 deletions application/controllers/EventRulesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,10 @@ public function indexAction(): void
'notifications/event-rules/add',
'plus'
))->setBaseTarget('_next')
->addAttributes(['class' => 'new-event-rule'])
Copy link
Member

Choose a reason for hiding this comment

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

Please also remove the class then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✅

->addAttributes(['class' => 'add-new-component'])
);

$this->content->getAttributes()->add(['class' => 'full-width']);
$this->addContent(new EventRuleList($eventRules));

if (! $searchBar->hasBeenSubmitted() && $searchBar->hasBeenSent()) {
Expand All @@ -104,7 +105,7 @@ public function indexAction(): void

public function addAction(): void
{
$this->addTitleTab(t('Add Event Rule'));
$this->addTitleTab(t('New Event Rule'));
$this->getTabs()->setRefreshUrl(Url::fromPath('notifications/event-rules/add'));

$this->controls->addAttributes(['class' => 'event-rule-detail']);
Expand Down
1 change: 1 addition & 0 deletions application/controllers/EventsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public function indexAction(): void
$eventList->setPageNumber($page);
}

$this->content->getAttributes()->add(['class' => 'full-width']);
if ($compact && $page > 1) {
$this->document->addFrom($eventList);
} else {
Expand Down
1 change: 1 addition & 0 deletions application/controllers/IncidentsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public function indexAction(): void
$this->addControl($limitControl);
$this->addControl($searchBar);

$this->content->getAttributes()->add(['class' => 'full-width']);
$this->addContent(new IncidentList($incidents));

if (! $searchBar->hasBeenSubmitted() && $searchBar->hasBeenSent()) {
Expand Down
4 changes: 2 additions & 2 deletions application/forms/ChannelForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ protected function assemble()
'submit',
[
'label' => $this->channelId === null ?
$this->translate('Add Channel') :
$this->translate('Create Channel') :
$this->translate('Save Changes')
]
);
Expand All @@ -167,7 +167,7 @@ protected function assemble()
'submit',
'delete',
[
'label' => $this->translate('Delete'),
'label' => $this->translate('Delete Channel'),
'class' => 'btn-remove',
'formnovalidate' => true
]
Expand Down
18 changes: 9 additions & 9 deletions application/forms/EntryForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,24 +285,24 @@ protected function assemble()
]);

$additionalButtons = [];
$cancelBtn = $this->createElement('submit', 'cancel', [
'label' => $this->translate('Cancel'),
'class' => 'btn-cancel',
'formnovalidate' => true
]);
$this->registerElement($cancelBtn);
$additionalButtons[] = $cancelBtn;

if ($this->showRemoveButton) {
$removeBtn = $this->createElement('submit', 'remove', [
'label' => $this->translate('Remove'),
'label' => $this->translate('Remove Entry'),
'class' => 'btn-remove',
'formnovalidate' => true
]);
$this->registerElement($removeBtn);
$additionalButtons[] = $removeBtn;
}

$cancelBtn = $this->createElement('submit', 'cancel', [
'label' => $this->translate('Cancel'),
'class' => 'btn-cancel btn-default',
'formnovalidate' => true
]);
$this->registerElement($cancelBtn);
$additionalButtons[] = $cancelBtn;

$this->getElement('submit')->prependWrapper((new HtmlDocument())->setHtmlContent(...$additionalButtons));

$this->addElement($this->createCsrfCounterMeasure(Session::getSession()->getId()));
Expand Down
2 changes: 1 addition & 1 deletion application/forms/SaveEventRuleForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public function setSubmitLabel(string $label): self
*/
public function getSubmitLabel(): string
{
return $this->submitLabel ?? t('Add Event Rule');
return $this->submitLabel ?? t('Create Event Rule');
}

/**
Expand Down
18 changes: 9 additions & 9 deletions application/forms/ScheduleForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,24 +122,24 @@ protected function assemble()
]);

$additionalButtons = [];
$cancelBtn = $this->createElement('submit', 'cancel', [
'label' => $this->translate('Cancel'),
'class' => 'btn-cancel',
'formnovalidate' => true
]);
$this->registerElement($cancelBtn);
$additionalButtons[] = $cancelBtn;

if ($this->showRemoveButton) {
$removeBtn = $this->createElement('submit', 'remove', [
'label' => $this->translate('Remove'),
'label' => $this->translate('Delete Schedule'),
'class' => 'btn-remove',
'formnovalidate' => true
]);
$this->registerElement($removeBtn);
$additionalButtons[] = $removeBtn;
}

$cancelBtn = $this->createElement('submit', 'cancel', [
'label' => $this->translate('Cancel'),
'class' => 'btn-cancel btn-default',
'formnovalidate' => true
]);
$this->registerElement($cancelBtn);
$additionalButtons[] = $cancelBtn;

$this->getElement('submit')->prependWrapper((new HtmlDocument())->setHtmlContent(...$additionalButtons));

$this->addElement($this->createCsrfCounterMeasure(Session::getSession()->getId()));
Expand Down
4 changes: 2 additions & 2 deletions library/Notifications/Web/Form/ContactForm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ protected function assemble()
'submit',
[
'label' => $this->contactId === null ?
$this->translate('Add Contact') :
$this->translate('Create Contact') :
$this->translate('Save Changes')
]
);
Expand All @@ -166,7 +166,7 @@ protected function assemble()
'submit',
'delete',
[
'label' => $this->translate('Delete'),
'label' => $this->translate('Delete Contact'),
'class' => 'btn-remove',
'formnovalidate' => true
]
Expand Down
1 change: 0 additions & 1 deletion library/Notifications/Widget/ItemList/EventListItem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,5 @@ protected function assembleMain(BaseHtmlElement $main): void
{
$main->add($this->createHeader());
$main->add($this->createCaption());
$main->add($this->createFooter());
}
}
2 changes: 1 addition & 1 deletion library/Notifications/Widget/Schedule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public function assemble()
new Link(
[
new Icon('plus'),
t('Add new entry')
t('Add Entry')
Copy link
Member

Choose a reason for hiding this comment

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

You've standardized it everywhere else to New ..., why not here??

Copy link
Contributor Author

@flourish86 flourish86 May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

So the idea behind this is:

  • You create/delete an Object (e.g. "New Schedule”), when you’re in the “schedules” views (Create)
  • You add/remove “entry" objects to that “schedule", when you’re in the “schedules” view (Create and link)

This helps

  • to clarify the different relations and
  • with distinguishing the different actions better, especially in a view with many buttons in the same design

I decided against additionally highlighting the “add entry” button as a primary button, because it’s not that important in that view and would distract from the actual content which is the schedule itself.

Screenshot 2023-05-23 at 13 27 30

Copy link
Member

Choose a reason for hiding this comment

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

Ah, okay, but I also add a new rule to the list of already existing event rules. I also add a new contact to the list of available contacts. In both cases, the relation is the list on the left. And to a list you add new items. The perfect label would be Add New Contact, but that's a bit redundant IMHO.

So if you think Add Entry is fine on the schedules view, I don't understand why you think Add Contact is not, on the contacts list.

Copy link
Contributor Author

@flourish86 flourish86 May 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, don't you see the difference?

contact (list): new contact
schedule (list): new schedule
schedule: add entry

Not sure, how to make it clearer.

It also helps clarify the difference when there's that many different buttons of the same style, as shown on the screenshot.

I agree on the redundancy, especially as it doesn't add any value and distracts from the actual important term "Contact".

I don't want to discuss this to death. If you insist or it doesn't make sense to you, we can label it "+ New Entry".

],
Url::fromPath('notifications/schedule/add-entry', ['schedule' => $this->schedule->id]),
['class' => 'button-link']
Expand Down
4 changes: 0 additions & 4 deletions public/css/calendar.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@
column-gap: .5em;
overflow: hidden;

time {
margin-right: .5em;
}

.attendee {
word-break: break-word;
}
Expand Down
7 changes: 7 additions & 0 deletions public/css/common.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
display: block;
.box-shadow(0, 0, 0, 1px, @gray-lighter);
}

.item-list .list-item {
.main, .visual {
padding-top: 0;
padding-bottom: 0;
}
}
}

.source-icon {
Expand Down
16 changes: 9 additions & 7 deletions public/css/detail/event-rule-detail.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,10 @@
&:first-child:before {
content: "";
display: block;
top: 50%;
}

&:first-child:last-child:before {
top: calc(~"50% - 1em");
}

Expand Down Expand Up @@ -202,7 +206,7 @@
}

.cache-notice {
margin: 1em;
margin: 1em 0;
padding: 1em;
background-color: @gray-lighter;
text-align: center;
Expand Down Expand Up @@ -358,10 +362,6 @@
width: 14.5em;
}

.new-event-rule {
margin-bottom: 1em;
}

.event-rule-and-save-forms {
display: flex;
flex-wrap: wrap;
Expand Down Expand Up @@ -402,10 +402,11 @@
margin-left: 1em;

&:disabled {
background: @gray-light;
background: @gray-lighter;
color: @disabled-gray;
cursor: not-allowed;
border-color: transparent;
opacity: .5;
}

&.btn-remove {
Expand All @@ -428,9 +429,10 @@
.remove-escalation-form {
button[disabled] {
&:disabled {
background: @gray-light;
background: @gray-lighter;
color: @disabled-gray;
cursor: not-allowed;
opacity: .5;
}
}
}
7 changes: 3 additions & 4 deletions public/css/event-source-badge.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,15 @@
}

.name {
position: relative;
display: inline-block;
height: 2em;
line-height: 2 - 2 * @ball-pad;
border-bottom-right-radius: 1em;
border-top-right-radius: 1em;
z-index: -99;
padding: @ball-pad .5em @ball-pad 1.5em;
margin-left: 1em;

background-color: @gray;
color: @text-color-inverted;
background-color: @gray-lighter;
color: @text-color;
}
}
26 changes: 26 additions & 0 deletions public/css/form.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,14 @@
border-radius: 0;
border: none;
}

.form-controls {
margin-top: 2em;

.btn-remove {
margin-right: auto;
}
}
}

.entry-form {
Expand Down Expand Up @@ -77,3 +85,21 @@
content: "\f0c0";
}
}

.icinga-form {
.form-controls {
.btn-remove:not(:hover) {
border-color: transparent;
}

.btn-default {
background: @low-sat-blue;
color: @icinga-blue;
border-color: transparent;

&:hover {
background: @low-sat-blue-dark;
}
}
}
}
4 changes: 4 additions & 0 deletions public/css/list/list-item.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,8 @@
line-height: 1.5*.857em;
}
}

.meta {
white-space: nowrap;
}
}