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

Improve linter messages in UI #4351

Merged
merged 11 commits into from
Nov 11, 2024
Merged

Improve linter messages in UI #4351

merged 11 commits into from
Nov 11, 2024

Conversation

xoxys
Copy link
Member

@xoxys xoxys commented Nov 11, 2024

  • Render linter errors as markdown string and convert it to HTML in frontend
  • Unify wording a bit
  • Restructure grid to improve mobile view
  • Use existing DocsLink component

Before:

image

After:

image

@xoxys xoxys requested a review from a team November 11, 2024 00:37
@6543 6543 added server ui frontend related ux user experience labels Nov 11, 2024
@6543
Copy link
Member

6543 commented Nov 11, 2024

tests fail 😅

@qwerty287
Copy link
Contributor

Do we have to use markdown? Can't we just directly use HTML so we don't need an additional parser in the frontend?

@xoxys
Copy link
Member Author

xoxys commented Nov 11, 2024

The error messages will also be used in CLI IIRC. So while the backticks also look ok on the CLI HTML does not.

@xoxys
Copy link
Member Author

xoxys commented Nov 11, 2024

I would also like to avoid the possibility to inject complex HTML (classes etc.) from the backend in the frontend, so I think markdown supports basic formatting while we don't need to worry about injected HTML breaks the frontend formatting. But that might be my personal impression only.

@qwerty287
Copy link
Contributor

OK get your point, but I somehow worry about the size of the frontend as it increases and increases... Yes, probably this one is not the big problem but still it's better to use native features.

@xoxys
Copy link
Member Author

xoxys commented Nov 11, 2024

Agree with that. What should we do in this particular case? IMO, properly formatting the linter messages in the UI is valuable and makes it more readable.

@xoxys xoxys force-pushed the improve-ui-linter branch from d2ba381 to 575234c Compare November 11, 2024 07:54
@xoxys
Copy link
Member Author

xoxys commented Nov 11, 2024

The remaining test failure looks unrelated to my code changed? 🤔 Fixed by #4353

@xoxys xoxys force-pushed the improve-ui-linter branch from 575234c to 858c273 Compare November 11, 2024 09:50
Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.

Project coverage is 27.32%. Comparing base (04e8309) to head (ccd7d36).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pipeline/frontend/yaml/linter/linter.go 82.35% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4351   +/-   ##
=======================================
  Coverage   27.32%   27.32%           
=======================================
  Files         379      379           
  Lines       27757    27757           
=======================================
  Hits         7584     7584           
  Misses      19488    19488           
  Partials      685      685           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xoxys xoxys changed the title Improve linter messages in UI WIP: Improve linter messages in UI Nov 11, 2024
@xoxys xoxys changed the title WIP: Improve linter messages in UI Improve linter messages in UI Nov 11, 2024
@xoxys xoxys marked this pull request as draft November 11, 2024 14:59
@xoxys xoxys marked this pull request as ready for review November 11, 2024 21:39
@xoxys xoxys requested a review from a team November 11, 2024 21:39
@6543 6543 merged commit a1193f0 into main Nov 11, 2024
9 checks passed
@6543 6543 deleted the improve-ui-linter branch November 11, 2024 22:34
@woodpecker-bot
Copy link
Collaborator

@woodpecker-bot woodpecker-bot mentioned this pull request Nov 11, 2024
1 task
@6543 6543 added the enhancement improve existing features label Nov 11, 2024
@woodpecker-bot woodpecker-bot mentioned this pull request Dec 14, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features server ui frontend related ux user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants