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

feat: tup 584 redesign news metadata #773

Merged
merged 15 commits into from
Mar 15, 2024
Merged

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Dec 14, 2023

Overview

Redesign news metadata.

Status

  • fix bug with short category, long author, and any date
    Leaving this as is. I do not want to refactor CSS grid to flex for this tiny bit of space. There is already a news CSS migration and a planned news CSS refactor coming. I can revisit then.
  • get designer review and approval

Related

Changes

See all PRs merged to this branch.

Testing

  1. Create article.
  2. View article on list page.
  3. View article page.
  4. Test both pages responsiveness.
  5. Confirm I get designer approval.

UI

Article Page

Before After
page before page after - desktop
Mouse
e.g. typical Desktop
Touch
e.g. typical Mobile
page after - desktop page after - mobile
Share.Links.on.Hover.mov

Article List Item

Before After
list before list after

wesleyboar and others added 5 commits December 14, 2023 10:18
* feat: TUP-553 hide byline prefix

* docs: TUP-553 simpler byline comments

* feat: TUP-553 new byline layout

* feat: TUP-553 new byline layout, page vs list
* feat: TUP-568 new date layout, split date and byline

* fix: TUP-568 revert published text change (diff ticket will do this)

* fix: TUP-568 revert social icons whitespace fix (diff PR will do this)
* feat: TUP-568 new date layout, split date and byline

* fix: TUP-568 revert published text change (diff ticket will do this)

* fix: TUP-568 revert social icons whitespace fix (diff PR will do this)

* feat: TUP-568 match design of time, cleanup organizational comments
@wesleyboar wesleyboar changed the title Feat/tup 584 news metadata feat: tup 584 redesign news metadata Dec 14, 2023
@wesleyboar wesleyboar marked this pull request as ready for review December 14, 2023 19:13
Copy link
Member Author

@wesleyboar wesleyboar left a comment

Choose a reason for hiding this comment

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

Notes for reviewers.

Comment on lines -56 to -61

#: djangocms-blog
#: templates/djangocms_blog/includes/blog_meta.html:6
msgid "by"
msgstr "by:"

Copy link
Member Author

Choose a reason for hiding this comment

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

But… but we still use trans "by" in Core-CMS blog_meta.html… ?! Yes. This deletion just removes our custom text for it, and uses "by" as is.

Comment on lines -44 to -49
grid-template-areas:
'cats'
'tags'
'attr'
'head'
'subh';
Copy link
Member Author

Choose a reason for hiding this comment

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

Why?! Instead of defining the layout of metadata once here, I define it for both …item.css and …page.css.

Comment on lines +93 to 96
/* To always hide byline prefix */
.app-blog .byline > span {
display: none;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Why even have trans "by" if you're not gonna use it? So another site can choose to show it, designers can change their mind, and I can continue to match the template I cloned and overwrite from the blog app1.

Footnotes

  1. Until I finally give in and make it my own, because the default template is not gospel, Wes; it's just an example.

@wesleyboar
Copy link
Member Author

Warning

Given a short category name and a longer author name, the date will not be next to the category name:

short category, long author, and any date - too much space

@wesleyboar wesleyboar merged commit 1874790 into main Mar 15, 2024
@wesleyboar wesleyboar deleted the feat/tup-584-news-metadata branch March 15, 2024 18:48
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.

None yet

1 participant