Style the silences list#809
Conversation
|
Fixes issue #808 |
|
Looks great!
…On Sun, May 21, 2017, 1:10 AM Max Inden ***@***.***> wrote:
Fixes issue #808 <#808>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#809 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEuA8gtnu1DygGvkIju9pGs46NXHuSH7ks5r73LOgaJpZM4NhUvp>
.
|
|
@fabxc thanks! I have another idea of how to improve the design. It requires changing the page background to gray, and highlighting the content blocks so they pop up. I would like to do it after this is merged. |
mxinden
left a comment
mxinden
left a comment
There was a problem hiding this comment.
Just some small things. Cool work!
| SilenceFormNewRoute _ -> | ||
| text "" | ||
|
|
||
| _ -> |
There was a problem hiding this comment.
To put this up for discussion: I would not render the "New Silence" button on the AlertList page, but only on the SilenceList page. This way interacting with silences is scoped to the silence related views. What do you think?
There was a problem hiding this comment.
But I do like the idea of moving the "New Silence" button to the NavBar.
There was a problem hiding this comment.
I thought about this:
- Silence is the only resource that is manageable from the UI
- Silence creation is one of the most important functions
- Navigating to the silences page just so that the button appears in the header feels strange
Why not have it as a shortcut?
There was a problem hiding this comment.
Sounds reasonable. I am fine with either one. Let's wait if anyone has any objections and then merge.
| a [ class <| "f6 link br1 ba mr1 mb2 dib " ++ classes ] | ||
| [ text content ] | ||
| labelButton : Maybe msg -> String -> Html msg | ||
| labelButton maybeMsg labelText = |
There was a problem hiding this comment.
👍 for making this consistent across pages.
| @@ -1,9 +1,9 @@ | |||
| module Views.Shared.SilenceBase exposing (view) | |||
| module Views.SilenceList.SilenceView exposing (view) | |||
There was a problem hiding this comment.
👍 Perfect, just like the AlertList view and AlertView. Thanks, forgot to change that in my previous PRs.
| ] | ||
| [ span [] [ text <| Utils.Date.timeFormat time ] | ||
| , small [] [ text <| Utils.Date.dateFormat time ] | ||
| [ text (string ++ " " ++ Utils.Date.timeFormat time ++ ", " ++ Utils.Date.dateFormat time) |
There was a problem hiding this comment.
Do we need to display the Silence state here? Isn't it already obvious by the new tabs in the Silence view?
There was a problem hiding this comment.
This is not to display the silence state, but more to give a hint of what timestamp we show (start time or end time).
| groupSilencesByState silences | ||
|
|
||
| tabView : State -> ( State, List a ) -> Html Msg | ||
| tabView currentState ( state, silences ) = |
| <link rel="stylesheet" href="https://unpkg.com/tachyons@4.5.3/css/tachyons.min.css"/> | ||
| <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-alpha.6/css/bootstrap.min.css" integrity="sha384-rwoIResjU2yc3z8GV/NPeZWAv56rSmLldC3R/AZzGRnGxQQKnKkoFVhFQhNUwEyJ" crossorigin="anonymous"> | ||
| <script src="https://use.fontawesome.com/b7508bb100.js"></script> | ||
| <style> |
There was a problem hiding this comment.
Wow, no custom css except couple of inline stylings
There was a problem hiding this comment.
Yeah, I think that we have enough stuff in bootstrap. It may be limiting, but in a good way, because it enforces consistent look and feel.
4 participants
I styled the silences list so it looks in sync with the alert list. I also deleted some code.
There seems to be a bug with pending silences #783