Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- The Conversation tab of a pull request on the Pulls screen shows each author's GitHub picture beside their reviews and comments, in a column down the left, the way GitHub does. Comments inside a review thread, on the Conversation tab and on the Files tab, are separated by a line. Until a picture loads, or for an account that has none, the author's initial stands in its place.
- Task lists in pull request descriptions and comments show a checkbox, ticked or not, in place of a literal `[x]` or `[ ]`.
- Top-level headings in pull request descriptions and comments are underlined across the text's width, as on GitHub.
- The Description heading on the Pulls screen's Overview has a ⋯ menu, as GitHub's comments do: Copy link copies the pull request's address, Copy Markdown its description as written, and Edit opens the description for editing, which the Edit button beside the heading did before.

### Changed

Expand Down
47 changes: 32 additions & 15 deletions native/cmd/lich-native/pulls_edit.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package main

import (
"io"
"strings"
"time"

"gioui.org/font"
"gioui.org/io/clipboard"
"gioui.org/layout"
"gioui.org/widget"

Expand Down Expand Up @@ -31,7 +34,8 @@ type editState struct {
body widget.Editor
bodySize ui.TextareaResize
bodyBar widget.Scrollbar
bodyOpen widget.Clickable
bodyMore widget.Clickable
bodyMenu ui.MenuState
bodySave widget.Clickable
bodyCancel widget.Clickable

Expand Down Expand Up @@ -118,33 +122,46 @@ func (v *pullView) pullTitle(gtx C, th *ui.Theme, p *pullsScreen, d backend.PRDe
)
}

// descriptionHeading is the Description heading, with the Edit that turns the
// description into a field while it is not one already.
func (v *pullView) descriptionHeading(gtx C, th *ui.Theme, d backend.PRDetail) D {
// descriptionHeading is the Description heading, with the ⋯ that copies the
// description or turns it into a field while it is not one already.
func (v *pullView) descriptionHeading(gtx C, th *ui.Theme, p *pullsScreen, d backend.PRDetail) D {
s := &v.edit
// Cancel is the field's, drawn under the heading: taken here, the
// heading and the field both read it.
if s.bodyCancel.Clicked(gtx) && v.busyNow() != "body" {
s.bodyURL = ""
}
if s.bodyOpen.Clicked(gtx) {
s.bodyURL = d.URL
s.body.SetText(d.Body)
gtx.Execute(keyFocus(&s.body))
}
heading := sectionHeading(th, i18n.T("pulls.pullsOverview.description"))
if s.bodyURL == d.URL {
return heading(gtx)
}
return layout.Flex{Alignment: layout.Middle}.Layout(gtx,
layout.Rigid(heading),
ui.Gap(2),
var copied string
entries := []ui.MenuEntry{
{Label: i18n.T("pulls.pullsOverview.copyLink"), OnClick: func() { copied = d.URL }},
{Label: i18n.T("pulls.pullsOverview.copyMarkdown"), Disabled: strings.TrimSpace(d.Body) == "", OnClick: func() { copied = d.Body }},
{Kind: ui.MenuSeparator},
{Label: i18n.T("pulls.pullsOverview.editDescription"), OnClick: func() {
s.bodyURL = d.URL
s.body.SetText(d.Body)
}},
}
b := th.Button(&s.bodyMore, "")
b.Icon, b.Variant, b.Size = icPullMore, ui.VariantGhost, ui.SizeIconXS
dims := layout.Flex{Alignment: layout.Middle}.Layout(gtx,
layout.Flexed(1, heading),
layout.Rigid(func(gtx C) D {
b := th.Button(&s.bodyOpen, i18n.T("pulls.pullsOverview.editDescription"))
b.Icon, b.Variant, b.Size = icPullEdit, ui.VariantGhost, ui.SizeXS
return b.Layout(gtx)
return v.withTip(gtx, th, p, "description", i18n.T("pulls.reviewThread.more"), func(gtx C) D {
return th.DropdownMenu(p.root, &s.bodyMenu, entries).Layout(gtx, b.Layout)
})
}),
)
if copied != "" {
gtx.Execute(clipboard.WriteCmd{Type: "application/text", Data: io.NopCloser(strings.NewReader(copied))})
}
if s.bodyURL == d.URL {
gtx.Execute(keyFocus(&s.body))
}
return dims
}

// descriptionField is the description being edited, raw Markdown, saved with
Expand Down
25 changes: 24 additions & 1 deletion native/cmd/lich-native/pulls_markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"math"

"gioui.org/font"
"gioui.org/layout"
"gioui.org/op"
"gioui.org/text"
"gioui.org/unit"
Expand All @@ -19,6 +20,9 @@ const (
markdownH1 = unit.Sp(20)
markdownH2 = unit.Sp(19)
markdownLineHeightScale = 1.5
// markdownHeadingLineHeightScale is GitHub's line-height for headings,
// which keeps an h1 or h2's rule close to its text.
markdownHeadingLineHeightScale = 1.25
)

func markdownProseColor(th *ui.Theme) color.NRGBA {
Expand All @@ -43,13 +47,18 @@ type markdownBlock struct {
prose *markdownCell
table *markdownTable
list *markdownList
// rule underlines the prose across the body's width, GitHub's border
// under an h1 or h2.
rule bool
}

// markdownCell is rendered Markdown and the link state it is laid out with.
type markdownCell struct {
spans []richtext.SpanStyle
code []bool
state richtext.InteractiveText
// lineHeight is the line height scale, markdownLineHeightScale when 0.
lineHeight float32
}

// markdownCache keeps each body's markdownText by a key of the caller's, so
Expand Down Expand Up @@ -144,6 +153,13 @@ func (b markdownBlock) layout(gtx C, th *ui.Theme, open func(url string)) D {
if b.list != nil {
return b.list.layout(gtx, th, open)
}
if b.rule {
return layout.Flex{Axis: layout.Vertical}.Layout(gtx,
layout.Rigid(func(gtx C) D { return b.prose.layout(gtx, th, open) }),
ui.VGap(1),
layout.Rigid(th.Separator),
)
}
return b.prose.layout(gtx, th, open)
}

Expand All @@ -158,7 +174,14 @@ func (c *markdownCell) layout(gtx C, th *ui.Theme, open func(url string)) D {
}
}
style := richtext.Text(&c.state, th.Shaper, c.spans...)
style.LineHeightScale = markdownLineHeightScale
style.LineHeightScale = c.lineHeightScale()
c.codeBackgrounds(gtx, th)
return style.Layout(gtx)
}

func (c *markdownCell) lineHeightScale() float32 {
if c.lineHeight != 0 {
return c.lineHeight
}
return markdownLineHeightScale
}
2 changes: 1 addition & 1 deletion native/cmd/lich-native/pulls_markdown_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func (c *markdownCell) codeBackgrounds(gtx C, th *ui.Theme) {
spans[i] = styledtext.SpanStyle{Font: span.Font, Size: span.Size, Color: color.NRGBA{}, Content: span.Content}
}
style := styledtext.Text(th.Shaper, spans...)
style.LineHeightScale = markdownLineHeightScale
style.LineHeightScale = c.lineHeightScale()
style.Layout(gtx, func(gtx layout.Context, i int, dims layout.Dimensions) {
if !c.code[i] || dims.Size.X == 0 || dims.Size.Y == 0 {
return
Expand Down
53 changes: 52 additions & 1 deletion native/cmd/lich-native/pulls_markdown_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ import (

"gioui.org/font"
"gioui.org/layout"
"gioui.org/op"
"gioui.org/widget"
"gioui.org/x/richtext"
"github.com/yuin/goldmark/ast"
extast "github.com/yuin/goldmark/extension/ast"

"github.com/lichdotdev/lich/native/ui"
)
Expand All @@ -18,6 +21,9 @@ type markdownList struct {

type markdownListItem struct {
marker markdownCell
// task is a task list item's box, drawn in the marker's place; nil
// for any other item.
task *widget.Bool
blocks []markdownBlock
}

Expand All @@ -33,6 +39,9 @@ func renderList(th *ui.Theme, source []byte, list *ast.List) *markdownList {
entry := markdownListItem{marker: markdownCell{spans: []richtext.SpanStyle{{
Font: font.Font{Typeface: th.Sans}, Size: markdownBodySize, Color: markdownProseColor(th), Content: marker,
}}}}
if box := taskCheckBox(item); box != nil {
entry.task = &widget.Bool{Value: box.IsChecked}
}
for child := item.FirstChild(); child != nil; child = child.NextSibling() {
if block, ok := renderBlock(th, source, child); ok {
entry.blocks = append(entry.blocks, block)
Expand All @@ -43,10 +52,21 @@ func renderList(th *ui.Theme, source []byte, list *ast.List) *markdownList {
return out
}

// taskCheckBox is the [ ] or [x] that opens item's first line, if it opens
// with one.
func taskCheckBox(item ast.Node) *extast.TaskCheckBox {
first := item.FirstChild()
if first == nil {
return nil
}
box, _ := first.FirstChild().(*extast.TaskCheckBox)
return box
}

func (l *markdownList) layout(gtx C, th *ui.Theme, open func(string)) D {
markerWidth := 0
for i := range l.items {
markerWidth = max(markerWidth, measureCell(gtx, th, &l.items[i].marker))
markerWidth = max(markerWidth, l.items[i].markerWidth(gtx, th))
}
children := make([]layout.FlexChild, 0, len(l.items))
for i := range l.items {
Expand All @@ -62,13 +82,44 @@ func (item *markdownListItem) layout(gtx C, th *ui.Theme, open func(string), mar
return layout.Flex{Alignment: layout.Start}.Layout(gtx,
layout.Rigid(func(gtx C) D {
gtx.Constraints.Min.X, gtx.Constraints.Max.X = markerWidth, markerWidth
if item.task != nil {
return item.layoutTask(gtx, th)
}
return layout.E.Layout(gtx, func(gtx C) D { return item.marker.layout(gtx, th, nil) })
}),
layout.Rigid(layout.Spacer{Width: ui.Space(2)}.Layout),
layout.Flexed(1, func(gtx C) D { return item.layoutBody(gtx, th, open) }),
)
}

// markerWidth is the width the item's marker, or its box, takes.
func (item *markdownListItem) markerWidth(gtx C, th *ui.Theme) int {
if item.task == nil {
return measureCell(gtx, th, &item.marker)
}
call := op.Record(gtx.Ops)
gtx.Constraints.Min = image.Point{}
dims := item.layoutTask(gtx, th)
call.Stop()
return dims.Size.X
}

// layoutTask is the item's box, read-only as GitHub draws it, centred on the
// first line.
func (item *markdownListItem) layoutTask(gtx C, th *ui.Theme) D {
box := th.Checkbox(item.task)
box.Disabled = true
line := int(float32(gtx.Sp(markdownBodySize)) * markdownLineHeightScale)
return layout.E.Layout(gtx, func(gtx C) D {
call := op.Record(gtx.Ops)
dims := box.Layout(gtx)
c := call.Stop()
defer op.Offset(image.Pt(0, (line-dims.Size.Y)/2)).Push(gtx.Ops).Pop()
c.Add(gtx.Ops)
return D{Size: image.Pt(dims.Size.X, line)}
})
}

func (item *markdownListItem) layoutBody(gtx C, th *ui.Theme, open func(string)) D {
children := make([]layout.FlexChild, 0, len(item.blocks)*2)
for i, block := range item.blocks {
Expand Down
28 changes: 28 additions & 0 deletions native/cmd/lich-native/pulls_markdown_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,31 @@ func TestMarkdownListWrapsInsideMarkerColumn(t *testing.T) {
t.Errorf("list size %v does not include marker column and wrapped body %v", got, bodySize)
}
}

func TestMarkdownTaskItemsDrawABoxInTheMarkersPlace(t *testing.T) {
list := renderBlocks(markdownTheme(), "- [x] done\n- [ ] todo\n- plain")[0].list
if len(list.items) != 3 {
t.Fatalf("got %d items, want 3", len(list.items))
}
for i, want := range []struct {
task, checked bool
text string
}{{true, true, "done"}, {true, false, "todo"}, {false, false, "plain"}} {
item := list.items[i]
if (item.task != nil) != want.task || (item.task != nil && item.task.Value != want.checked) {
t.Errorf("item %d: task %v, want task %v checked %v", i, item.task, want.task, want.checked)
}
var text string
for _, span := range item.blocks[0].prose.spans {
text += span.Content
}
if text != want.text {
t.Errorf("item %d reads %q, want %q", i, text, want.text)
}
}
gtx := layout.Context{Ops: new(op.Ops), Metric: unit.Metric{PxPerDp: 1, PxPerSp: 1},
Constraints: layout.Constraints{Max: image.Pt(400, 800)}}
if w := list.items[0].markerWidth(gtx, markdownTheme()); w < gtx.Dp(ui.Space(4)) {
t.Errorf("marker column %dpx is narrower than the box", w)
}
}
6 changes: 5 additions & 1 deletion native/cmd/lich-native/pulls_markdown_render.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
// markdownURL is the span metadata a link's destination is kept under.
const markdownURL = "url"

var markdownParser = goldmark.New(goldmark.WithExtensions(extension.Table, extension.Linkify)).Parser()
var markdownParser = goldmark.New(goldmark.WithExtensions(extension.Table, extension.Linkify, extension.TaskList)).Parser()

// renderBlocks parses src once, so a reference link resolves wherever its
// definition sits, and renders each top-level node as its own block.
Expand All @@ -48,6 +48,10 @@ func renderBlock(th *ui.Theme, source []byte, n ast.Node) (markdownBlock, bool)
return markdownBlock{table: renderTable(th, source, n)}, true
case *ast.List:
return markdownBlock{list: renderList(th, source, n)}, true
case *ast.Heading:
cell := renderNode(th, source, n, markdownProseColor(th))
cell.lineHeight = markdownHeadingLineHeightScale
return markdownBlock{prose: cell, rule: n.Level <= 2}, len(cell.spans) > 0
default:
cell := renderNode(th, source, n, markdownProseColor(th))
return markdownBlock{prose: cell}, len(cell.spans) > 0
Expand Down
15 changes: 14 additions & 1 deletion native/cmd/lich-native/pulls_markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package main

import (
"image"
"slices"
"strings"
"testing"
"time"
Expand All @@ -17,7 +18,8 @@ import (
)

func markdownTheme() *ui.Theme {
return &ui.Theme{Palette: ui.Dark, Shaper: text.NewShaper(text.WithCollection(gofont.Collection()))}
th := &ui.Theme{Palette: ui.Dark, Shaper: text.NewShaper(text.WithCollection(gofont.Collection()))}
return th.On(th.Background)
}

func markdownFrame(height int) (C, *op.Ops) {
Expand Down Expand Up @@ -149,3 +151,14 @@ func TestMarkdownUpdatesWhenInterfaceAndEditorFontsChange(t *testing.T) {
t.Fatal("shaper change retained the old measurements")
}
}

func TestMarkdownRulesUnderTopHeadingsOnly(t *testing.T) {
blocks := renderBlocks(markdownTheme(), "# One\n\n## Two\n\n### Three\n\nText")
var got []bool
for _, b := range blocks {
got = append(got, b.rule)
}
if want := []bool{true, true, false, false}; !slices.Equal(got, want) {
t.Errorf("rules = %v, want %v", got, want)
}
}
6 changes: 3 additions & 3 deletions native/cmd/lich-native/pulls_panels.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ var (

// overview is the description beside the pull request's facts.
func (v *pullView) overview(gtx C, th *ui.Theme, p *pullsScreen, d backend.PRDetail, act *pullsAction) D {
rows := v.overviewRows(th, d, act)
rows := v.overviewRows(th, p, d, act)
if gtx.Constraints.Max.X < gtx.Dp(overviewSplitWidth) {
facts := func(gtx C) D {
return layout.Inset{Left: pullsPadX, Right: pullsPadX, Top: ui.Space(4)}.Layout(gtx, func(gtx C) D { return v.facts(gtx, th, p, d) })
Expand Down Expand Up @@ -330,7 +330,7 @@ func (v *pullView) checkoutFact(th *ui.Theme, p *pullsScreen, d backend.PRDetail

// overviewRows is the description under its heading, held to a reading
// width.
func (v *pullView) overviewRows(th *ui.Theme, d backend.PRDetail, act *pullsAction) []layout.Widget {
func (v *pullView) overviewRows(th *ui.Theme, p *pullsScreen, d backend.PRDetail, act *pullsAction) []layout.Widget {
pad := func(top float32, w layout.Widget) layout.Widget {
return func(gtx C) D {
gtx.Constraints.Max.X = min(gtx.Constraints.Max.X, gtx.Dp(overviewProseWidth+2*pullsPadX))
Expand All @@ -339,7 +339,7 @@ func (v *pullView) overviewRows(th *ui.Theme, d backend.PRDetail, act *pullsActi
}
body := strings.TrimSpace(d.Body)
return []layout.Widget{
pad(4, func(gtx C) D { return v.descriptionHeading(gtx, th, d) }),
pad(4, func(gtx C) D { return v.descriptionHeading(gtx, th, p, d) }),
pad(2, func(gtx C) D {
if v.edit.bodyURL == d.URL {
return v.descriptionField(gtx, th, d, act)
Expand Down
Loading
Loading