In the monolith we inherit this property from a CSS reset, but we cannot
count on that being present in every application using this library, so
also setting this property explicitly.
The table loading styles require the paddings that are removed for the
general table case. That's okay: the table loading styles do not support
putting custom content in cells, and so the option of setting custom
paddings for content will not be missed.
The previous removal of default table cell padding makes the default
alignment in content look weird. Adding a vertical alignment property
fixes this.
We previously removed the default padding because it did not play nice
with designs requiring less padding. Does the addition of vertical
alignment create a similar problem? I'd argue now. Padding cannot be
easily countered. Vertical alignment can easily be countered by
embedding div's in the cells with custom styling.
We've run into a situation in CCS where the default padding added to
cells are to large. Considering it is much harder to remove a padding
from the outside then it is to add one, this commit removes the default
cell padding for tables.
This way you're not limited to creating tables with fixed-width columns.
This also makes the table take up the maximum available width space by
default.
The table styles are currently depending on some browser styles. If
those are disabled (for instance using a reset.css file), then tables do
not look correct. This makes explicit styles that are currently coming
from the browser.
There are places where we need the select to be as wide as possible.
Since it seems like a sensible default to have all the selects expand,
we take that approach.
If we find out that this is harder, we can release use the old version.
We want to allow the control to take up the full width of the parent.
This will most likely break something if you try to upgrade directly.
Should be fixable by putting the control in an element with
the appropriate width.
tl;dr; Use a class for each variant instead of overriding one variant.
Before, we relied on CSS specificity in an unclear way.
The `Focused` class was applying properly because it was ordered later
than the `Tab` class in the stylesheet.
The ordering that is important is the ordering in `styles` value.
Since `elm-css` generates the stylesheet in the order of the lists,
the `Focused` rule would be generated after the `Tab` rule.
Meaning the `Focused` rule would take precedence over the `Tab` rule
if an element had both classes as it was defined later in the stylesheet.
There are some concerns with this approach:
1. It's not readily apparent that the ordering in `styles` is important.
It is pretty easy to change the ordering of the list
and have it break the styling.
2. We rely on `elm-css` to generate the stylesheet in a specific order.
If it changes the order of rules it generates,
we're almost surely going to break the styling.
3. Altering styles for tabs that are not focused is even less intuitive.
Since the specificity is the same,
you might not know why a given rule applies (or doesn't apply).
Rather, we can eschew the specificity/precedence issues
by applying a different class to each tab.
The stuff that is the same can stay on the `Tab` class,
and the stuff that differs can be on different classes.
We are now able to set the background color for `Unfocused` tabs.
We were relying on the control being placed atop a white background.
When we moved to using the control atop a non-whitebackground,
it showed that the `Unfocused` tabs had a transparent backround.
All of our designs show `Unfocused` tabs with a white backround.
See https://github.com/NoRedInk/noredink-ui/pull/14 for more information.
This change allows SegmentedControl to no longer needs to take a record
with all icon assets. It's the construction of the `IconType` that now
requires an asset, which one depends on the icon being constructed.
The purpose of this change is to allow each icon to list its asset
dependencies separately. This allows services consuming the icon library
to only provide those icons that are actually used.
This version is like `Nri.Ui.SegmentedControl.V1` with the icon changes.
It has the fix for `box-sizing`.
All the other caveats from `Nri.Ui.SegmentedControl.V1`
(if there were any) apply here as well.
The recommendation is to break the styles API rather than the view API
when moving something out of the monolith into this repo.
`Nri.Icon` is not really setup for that sort of breakage.
If we would prefer to have the styles break rather than the view,
that will take more work.
Work that can be done independent of the extraction.
The transition in the monolith ought to look something like:
```elm
module Nri.Icon exposing (..)
import Html exposing (Html)
import Nri.SvgSprite
import Nri.Ui.Icon.V1 exposing (Assets, IconType)
icon : { alt : String, icon : IconType } -> Html msg
icon config =
Nri.Ui.Icon.V1.icon assets
assets : Assets {}
assets =
{ activity = Nri.SvgSprite.activity
, arrowDown = Nri.SvgSprite.arrowDown
, attention_svg = Nri.Assets.attention_svg
...
}
```
So hopefully, the change is still very small on the monolith side.
There's maybe a bigger concern than which API breaks.
`Nri.Icon` has some behavior for a11y.
We could definitely change the internals over during the extraction.
But, since all of these changes are value-level changes,
it's very likely that we'll break something in the process.
That's a bigger concern because instead of affecting
the handful of Engineers working at NRI,
we would be affecting the millions of people using the site.
We shouldn't fear making those kinds of changes.
However, we should make them when we can give them the appropriate
attention they deserve.
Not when one person is trying to move as fast as possible to avoid
race conditions of moving modules between repos.
The `initial` `background-color` is `transparent`.
We got away with not setting the background color before
as we were always using segmented controls on top of white backgrounds.
We now want to use a segmented control atop a non-white background,
so we have to explicitly set the `background-color`.
tl;dr; Use a class for each variant instead of overriding one variant.
Before, we relied on CSS specificity in an unclear way.
The `Focused` class was applying properly because it was ordered later
than the `Tab` class in the stylesheet.
The ordering that is important is the ordering in `styles` value.
Since `elm-css` generates the stylesheet in the order of the lists,
the `Focused` rule would be generated after the `Tab` rule.
Meaning the `Focused` rule would take precedence over the `Tab` rule
if an element had both classes as it was defined later in the stylesheet.
There are some concerns with this approach:
1. It's not readily apparent that the ordering in `styles` is important.
It is pretty easy to change the ordering of the list
and have it break the styling.
2. We rely on `elm-css` to generate the stylesheet in a specific order.
If it changes the order of rules it generates,
we're almost surely going to break the styling.
3. Altering styles for tabs that are not focused is even less intuitive.
Since the specificity is the same,
you might not know why a given rule applies (or doesn't apply).
Rather, we can eschew the specificity/precedence issues
by applying a different class to each tab.
The stuff that is the same can stay on the `Tab` class,
and the stuff that differs can be on different classes.
On the monolith we have a base css file somewhere that adds this
property by default. In this package we don't (and even if we did, we
couldn't bundle it with the elm code), so this commit adds the property
manually on a div that needs it.
It's perhaps debateable if all these modules belong there, but it makes
sense as a first step, to avoid conflicts once we start importing this
from the monolith.
Because the textarea label overflowed its container it was drawn
seemingly closer to the content above it than user-set margins can
account for. This then requires the user to add additional wrappers and
margins to compensate. Drawing the textarea completely within its
enclosing container makes this unnecessary.
This is a necessity to allow doodads to incorporate assets like
background images into their css.
I've tried to make the change as backwards-compatible as possible,
leaving most of the existing funtions alone. The `.css` property on the
returned Styles instance still needs those asset paths though, so this
is still a major update.
This doesn't include the entire spec for the new outline styling, just
the part we currently need for content creation outline draggable
creation. The additions to complete the spec look doable in the current
structure though.
Still missing the assets.
Will have to figure out what to do in general for assets.
The concern is that we try to make assets content addressable
so we can cache them appropriately.
But we haven't done this in a place where we don't also own the assets.
