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.

Sign up for GitHub

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

Jump to bottom

Markdown support in the pinned items #31251

Merged
merged 82 commits into from
Jun 21, 2023
Merged

Markdown support in the pinned items #31251

merged 82 commits into from
Jun 21, 2023

Conversation

kamilmielnik
Copy link
Contributor

@kamilmielnik kamilmielnik commented Jun 1, 2023 •
edited

Closes #30381

I found the following issues while working on this:

Description

Adds markdown support in tooltips in pinned items.

How to verify

  1. Create a:
    • Dashboard
    • Model
    • Question
  2. Add description to it:
    • markdown with short / long text
    • plain text with short / long text
  3. Pin it to a dashboard
  4. If it's a question, turn the visualization on and off
  5. In the pinned item hover the text description (or description icon) to display the tooltip - it should show markdown without headings and links formatted
  6. If there is a description block in the card, it should contain only the first description line with markdown formatting removed

To verify skeletons, check out the description tooltip when card is loading. To keep it in that state forever you can block requests to api/card/*/query URL in your developer tools.

Demo

Before

before.mp4

After

after.mp4

@kamilmielnik
Allow ReactNode as tooltip in Ellipsified
aa83002 
- Consistently destructure all imports from react package
@kamilmielnik
Introduce getMarkdownFirstLine
f009c44
@kamilmielnik
- Extract markdownToHtml
74b40e6 
- Refactor getMarkdownFirstLine into getLeadingText
@kamilmielnik
Render description as markdown in StaticSkeleton
58aac9e
@kamilmielnik kamilmielnik self-assigned this Jun 1, 2023
@kamilmielnik kamilmielnik changed the title 30381 markdown in pinned items Markdown support in the pinned items Jun 1, 2023
@kamilmielnik
Cover getLeadingText with tests
ef6100c
@kamilmielnik
Consolidate tests
d8d8aa7
@kamilmielnik
Extract test-utils
6c15a13
@kamilmielnik
Add initial markdownToHtml tests
4bc4e44
@kamilmielnik
Add a test case for a mix of h1 and p
7f2ff59
@kamilmielnik
Rename markdownToHtml to convertMarkdownToHtml
195ccd1
@kamilmielnik
Rename convertMarkdownToHtml to renderMarkdownToHtml
8864c98
@kamilmielnik
Rename renderMarkdownToHtml to renderMarkdown
da3cb57
@kamilmielnik
Add a test case for alt text of an image
1849a3b 
- e.g. Gmail does this and it's not the desired behavior
@kamilmielnik
Place the tooltip at the bottom (as per design)
7532f8a
@kamilmielnik
Add StaticSkeleton initial test suite
4333571
@kamilmielnik
Add test case for not showing description tooltip when description is…
387cf44 
… fully rendered
@kamilmielnik
Rename renderMarkdown to parseMarkdown
832c019
@kamilmielnik
Format code
25c73aa
@kamilmielnik
Add an e2e test for markdown rendering in pinned models
073c807 
- Introduce changeCardDescription helper in e2e-collection-helpers
- Move openRootCollection to e2e-collection-helpers
@kamilmielnik
Add an e2e test for markdown rendering in pinned native questions
adcc6fb
@kamilmielnik
Remove redundant assertions
885b03a
@kamilmielnik
Add an e2e test for markdown rendering in pinned dashboards
1a4a779
@kamilmielnik
Remove exact: false
035278e
@kamilmielnik
Move getLeadingText & parseMarkdown to metabase/lib/markdown
8454951
@kamilmielnik
Add markdown tooltip to PinnedItemCard
445232b
@kamilmielnik
Add missing import
f4c01b3
@kamilmielnik
Add disallowHeading & unstyleLinks props to markdown rendered in tool…
7aeb00c 
…tips
@kamilmielnik
Copy link
Contributor Author

kamilmielnik commented Jun 16, 2023 •
edited

Is it solvable by white-space: pre?

pre breaks ellipsis but pre-line does the job, thank you!
I updated the description with new videos - now there's an ... shown when ellipsis is applied.
Ready for review @alxnddr.

alxnddr
alxnddr approved these changes Jun 16, 2023
View reviewed changes
Copy link
Member

@alxnddr alxnddr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Please check out a couple of minor comments before merging

frontend/src/metabase/dashboard/containers/DashboardHeader.jsx Outdated Show resolved Hide resolved
...nd/src/metabase/visualizations/components/skeletons/StaticSkeleton/StaticSkeleton.styled.tsx Outdated Show resolved Hide resolved
@kamilmielnik kamilmielnik enabled auto-merge (squash) June 21, 2023 06:23
@kamilmielnik kamilmielnik merged commit b92caa6 into master Jun 21, 2023
91 of 92 checks passed
@kamilmielnik kamilmielnik deleted the 30381-markdown-in-pinned-items branch June 21, 2023 08:29
@kamilmielnik kamilmielnik added the backport Automatically create PR on current release branch on merge label Jun 21, 2023
@github-actions
Copy link

@kamilmielnik could not create a backport due to conflicts

kamilmielnik added a commit that referenced this pull request Jun 21, 2023
@kamilmielnik
Manually backport #31251
74c9f3e
@kamilmielnik kamilmielnik mentioned this pull request Jun 21, 2023
Closed
This was referenced Jun 23, 2023
Closed
Merged
@kamilmielnik kamilmielnik mentioned this pull request Jul 5, 2023
Closed
@kamilmielnik kamilmielnik mentioned this pull request Jul 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ranquild ranquild ranquild approved these changes

@alxnddr alxnddr Awaiting requested review from alxnddr

Assignees

@kamilmielnik kamilmielnik

Labels
backport Automatically create PR on current release branch on merge visual Run Percy visual testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Markdown support in the pinned items
3 participants
@kamilmielnik @ranquild @alxnddr