Skip to content

Render fields as markdown #1747

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

Tschuppi81
Copy link
Contributor

@Tschuppi81 Tschuppi81 commented Mar 21, 2025

Directory: Render markdown in accordion layout

TYPE: Feature
LINK: ogc-2131

Copy link

linear bot commented Mar 21, 2025

Copy link

codecov bot commented Mar 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.26%. Comparing base (97478cf) to head (ac92189).
Report is 14 commits behind head on master.

Additional details and impacted files
Files with missing lines Coverage Δ
src/onegov/org/layout.py 91.81% <100.00%> (+0.09%) ⬆️

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97478cf...ac92189. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Daverball
Copy link
Member

Daverball commented Mar 25, 2025

@Tschuppi81 Did you check whether the other view type uses markdown? It might use to_html_ul, which should be sufficient for this use-case (it'll also add the correct css class for bulleted lists). Full markdown support seems like overkill to me.

@Daverball
Copy link
Member

Daverball commented Mar 25, 2025

@Tschuppi81 Did you check whether the other view type uses markdown? It might use to_html_ul, which should be sufficient for this use-case (it'll also add the correct css class for bulleted lists). Full markdown support seems like overkill to me.

Actually nevermind, this is default form display rendering. So it is probably a markdown field. I would suggest relying on the field display renderer for rendering values, rather than trying to roll your own custom display. Have a look at the display_fields macro. You will need some special handling for UploadField/UploadMultipleField but for the rest you should be able to rely on form.render_display(field).

Although you may actually just be able to use the display_fields macro directly and rely purely on CSS for any style changes between the two layouts.

@Tschuppi81
Copy link
Contributor Author

Yes, I will try to reuse the display_fields macro

@Tschuppi81 Tschuppi81 requested a review from Daverball March 26, 2025 11:31
Copy link
Member

@Daverball Daverball left a comment

Choose a reason for hiding this comment

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

LGTM.

I just have a small question regarding the CSS change.

padding: 0;
display: inline;
}

Copy link
Member

Choose a reason for hiding this comment

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

Why is there only a CSS change for Org, does Town6 not need one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fort town6 it was introduced with ogc-1634, ed93c7d

Copy link
Member

Choose a reason for hiding this comment

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

But this change is about .field-display not .accordion-title or .accordion-content. If anything you'd be missing CSS rules for both Org and Town now, just different ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now found a way to make it independent of .field-display

Copy link
Member

Choose a reason for hiding this comment

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

My problem is not with the rules you added or what elements they address.

I'm asking why you have custom CSS for the header and content of the accordion in Town6 and not Org, and why you have custom styling for the display of the fields in Org but not Town6.

If you're satisfied with how it looks in both styles I'm satisfied too.

I was merely puzzled by the difference between Org and Town6, since you've decided to manually style completely different elements in both cases. So I was asking whether or not that was intentional or if you accidentally forgot to sync the rules between Org and Town6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants