Skip to content

Commit

Permalink
feat: inline theme declaration in explore (#6015)
Browse files Browse the repository at this point in the history
* inline theme wip

* conditionally create resource

* remove comments

* remove comments

* err check

* test fix

* cleanup

* Review

* Improve comment

---------

Co-authored-by: Benjamin Egelund-Müller <b@egelund-muller.com>
  • Loading branch information
briangregoryholmes and begelundmuller authored Nov 5, 2024
1 parent db4bc11 commit ef7c408
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 28 deletions.
68 changes: 63 additions & 5 deletions runtime/compilers/rillv1/parse_explore.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"
"time"

"github.com/google/uuid"
runtimev1 "github.com/rilldata/rill/proto/gen/rill/runtime/v1"
"golang.org/x/exp/maps"
"gopkg.in/yaml.v3"
Expand All @@ -19,7 +20,7 @@ type ExploreYAML struct {
MetricsView string `yaml:"metrics_view"`
Dimensions *FieldSelectorYAML `yaml:"dimensions"`
Measures *FieldSelectorYAML `yaml:"measures"`
Theme string `yaml:"theme"`
Theme yaml.Node `yaml:"theme"` // Name (string) or inline theme definition (map)
TimeRanges []ExploreTimeRangeYAML `yaml:"time_ranges"`
TimeZones []string `yaml:"time_zones"`
Defaults *struct {
Expand Down Expand Up @@ -152,9 +153,14 @@ func (p *Parser) parseExplore(node *Node) error {
measuresSelector = tmp.Measures.Proto()
}

// Add theme to refs
if tmp.Theme != "" {
node.Refs = append(node.Refs, ResourceName{Kind: ResourceKindTheme, Name: tmp.Theme})
// Parse theme if present.
// If it returns a themeSpec, it will be inserted as a separate resource later in this function.
themeName, themeSpec, err := p.parseExploreTheme(node.Name, &tmp.Theme)
if err != nil {
return err
}
if themeName != "" {
node.Refs = append(node.Refs, ResourceName{Kind: ResourceKindTheme, Name: themeName})
}

// Build and validate time ranges
Expand Down Expand Up @@ -259,12 +265,64 @@ func (p *Parser) parseExplore(node *Node) error {
r.ExploreSpec.DimensionsSelector = dimensionsSelector
r.ExploreSpec.Measures = measures
r.ExploreSpec.MeasuresSelector = measuresSelector
r.ExploreSpec.Theme = tmp.Theme
r.ExploreSpec.Theme = themeName
r.ExploreSpec.TimeRanges = timeRanges
r.ExploreSpec.TimeZones = tmp.TimeZones
r.ExploreSpec.DefaultPreset = defaultPreset
r.ExploreSpec.EmbedsHidePivot = tmp.Embeds.HidePivot
r.ExploreSpec.SecurityRules = rules

if themeSpec != nil {
r, err := p.insertResource(ResourceKindTheme, themeName, node.Paths)
if err != nil {
// Normally we could return the error, but we can't do that here because we've already inserted the explore.
// Since the theme has been validated with insertDryRun in parseExploreTheme, this error should never happen in practice.
// So let's panic.
panic(err)
}
r.ThemeSpec = themeSpec
}

return nil
}

func (p *Parser) parseExploreTheme(exploreName string, n *yaml.Node) (string, *runtimev1.ThemeSpec, error) {
if n == nil || n.IsZero() {
return "", nil, nil
}

switch n.Kind {
case yaml.ScalarNode: // It's the name of an existing theme
var name string
err := n.Decode(&name)
if err != nil {
return "", nil, err
}
return name, nil, nil
case yaml.MappingNode: // It's an inline definition of a new theme
tmp := &ThemeYAML{}
err := n.Decode(tmp)
if err != nil {
return "", nil, err
}

name := fmt.Sprintf("%s--theme", exploreName)
err = p.insertDryRun(ResourceKindTheme, name)
if err != nil {
name = fmt.Sprintf("%s--theme-%s", exploreName, uuid.New())
err = p.insertDryRun(ResourceKindTheme, name)
if err != nil {
return "", nil, err
}
}

spec, err := p.parseThemeYAML(tmp)
if err != nil {
return "", nil, err
}

return name, spec, nil
default:
return "", nil, fmt.Errorf("invalid theme: should be a string or mapping, got kind %q", n.Kind)
}
}
48 changes: 25 additions & 23 deletions runtime/compilers/rillv1/parse_theme.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,39 +22,41 @@ func (p *Parser) parseTheme(node *Node) error {
return err
}

// Parse the colors now to get the parse error before inserting resource
var pc csscolorparser.Color
hasPc := false
var sc csscolorparser.Color
hasSc := false
if tmp.Colors.Primary != "" {
pc, err = csscolorparser.Parse(tmp.Colors.Primary)
if err != nil {
return err
}
hasPc = true
}
if tmp.Colors.Secondary != "" {
sc, err = csscolorparser.Parse(tmp.Colors.Secondary)
if err != nil {
return err
}
hasSc = true
spec, err := p.parseThemeYAML(tmp)
if err != nil {
return err
}

r, err := p.insertResource(ResourceKindTheme, node.Name, node.Paths, node.Refs...)
if err != nil {
return err
}

if hasPc {
r.ThemeSpec.PrimaryColor = toThemeColor(pc)
r.ThemeSpec = spec

return nil
}

func (p *Parser) parseThemeYAML(tmp *ThemeYAML) (*runtimev1.ThemeSpec, error) {
spec := &runtimev1.ThemeSpec{}

if tmp.Colors.Primary != "" {
pc, err := csscolorparser.Parse(tmp.Colors.Primary)
if err != nil {
return nil, err
}
spec.PrimaryColor = toThemeColor(pc)
}
if hasSc {
r.ThemeSpec.SecondaryColor = toThemeColor(sc)

if tmp.Colors.Secondary != "" {
sc, err := csscolorparser.Parse(tmp.Colors.Secondary)
if err != nil {
return nil, err
}
spec.SecondaryColor = toThemeColor(sc)
}

return nil
return spec, nil
}

func toThemeColor(c csscolorparser.Color) *runtimev1.Color {
Expand Down
49 changes: 49 additions & 0 deletions runtime/compilers/rillv1/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1503,12 +1503,27 @@ func TestTheme(t *testing.T) {
ctx := context.Background()
repo := makeRepo(t, map[string]string{
`rill.yaml`: ``,
// Theme resource
`themes/t1.yaml`: `
type: theme
colors:
primary: red
secondary: grey
`,
// Explore referencing the external theme resource
`explores/e1.yaml`: `
type: explore
metrics_view: missing
theme: t1
`,
// Explore that defines an inline theme
`explores/e2.yaml`: `
type: explore
metrics_view: missing
theme:
colors:
primary: red
`,
})

Expand All @@ -1531,6 +1546,40 @@ colors:
},
},
},
{
Name: ResourceName{Kind: ResourceKindExplore, Name: "e1"},
Paths: []string{"/explores/e1.yaml"},
Refs: []ResourceName{{Kind: ResourceKindMetricsView, Name: "missing"}, {Kind: ResourceKindTheme, Name: "t1"}},
ExploreSpec: &runtimev1.ExploreSpec{
MetricsView: "missing",
DimensionsSelector: &runtimev1.FieldSelector{Selector: &runtimev1.FieldSelector_All{All: true}},
MeasuresSelector: &runtimev1.FieldSelector{Selector: &runtimev1.FieldSelector_All{All: true}},
Theme: "t1",
},
},
{
Name: ResourceName{Kind: ResourceKindExplore, Name: "e2"},
Paths: []string{"/explores/e2.yaml"},
Refs: []ResourceName{{Kind: ResourceKindMetricsView, Name: "missing"}, {Kind: ResourceKindTheme, Name: "e2--theme"}},
ExploreSpec: &runtimev1.ExploreSpec{
MetricsView: "missing",
DimensionsSelector: &runtimev1.FieldSelector{Selector: &runtimev1.FieldSelector_All{All: true}},
MeasuresSelector: &runtimev1.FieldSelector{Selector: &runtimev1.FieldSelector_All{All: true}},
Theme: "e2--theme",
},
},
{
Name: ResourceName{Kind: ResourceKindTheme, Name: "e2--theme"},
Paths: []string{"/explores/e2.yaml"},
ThemeSpec: &runtimev1.ThemeSpec{
PrimaryColor: &runtimev1.Color{
Red: 1,
Green: 0,
Blue: 0,
Alpha: 1,
},
},
},
}

p, err := Parse(ctx, repo, "", "", "duckdb")
Expand Down

0 comments on commit ef7c408

Please sign in to comment.