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.

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

Styling for Apply/Discard buttons #21017

Merged
merged 22 commits into from
Nov 26, 2024
Merged

Styling for Apply/Discard buttons #21017

merged 22 commits into from
Nov 26, 2024

Conversation

rtfeldman
Copy link
Contributor

@rtfeldman rtfeldman commented Nov 21, 2024

Change the "Apply" and "Discard" buttons to match @danilo-leal's design! Here are some different states:

Cursor in the first hunk

Now that the cursor is in a particular hunk, we show the "Apply" and "Discard" names, and the keyboard shortcut. If I press the keyboard shortcut, it will only apply to this hunk.

Screenshot 2024-11-23 at 10 54 45 PM

Cursor in the second hunk

Moving the cursor to a different hunk changes which buttons get the keyboard shortcut treatment. Now the keyboard shortcut is shown next to the hunk that will actually be affected if you press that shortcut.

Screenshot 2024-11-23 at 10 56 27 PM

Release Notes:

  • Restyled Apply/Discard buttons

@rtfeldman rtfeldman marked this pull request as draft November 21, 2024 20:52
@rtfeldman rtfeldman force-pushed the apply-discard-styling branch from 5f992e8 to 44db64a Compare November 21, 2024 22:23
@notpeter notpeter added the cla-signed The user has signed the Contributor License Agreement label Nov 22, 2024
This test passed until 4ca64cc,
which caused it to fail. However, looking at the test, it appears
to have previously been broken (and 4ca64cc
fixed it).

In the test, we perform these two operations and then diff the result:

```rust
editor.select_up_by_lines(&SelectUpByLines { lines: 5 }, cx);
editor.delete_line(&DeleteLine, cx);
```

Previously, the test was asserting that the 4 lines (not 5) above
the cursor were deleted. Now we instead assert that all 5 lines
above the cursor were deleted, which seems correct.
@rtfeldman rtfeldman marked this pull request as ready for review November 26, 2024 16:09
@rtfeldman rtfeldman merged commit 8847480 into main Nov 26, 2024
14 checks passed
@rtfeldman rtfeldman deleted the apply-discard-styling branch November 26, 2024 16:09
maxbrunsfeld added a commit that referenced this pull request Nov 26, 2024
@Spoutnik97
Copy link

@rtfeldman where could we find designs ? Are they public ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants