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 617: Migrate c-card fixes to core-styles #251

Merged
merged 9 commits into from
Nov 2, 2023

Conversation

R-Tomas-Gonzalez
Copy link
Collaborator

@R-Tomas-Gonzalez R-Tomas-Gonzalez commented Oct 27, 2023

Overview

Migrate c-card style fixes to core-styles

Archived Notices about Button a Card

Warning
Should not be merged until the code is adapted to not conflict with Bootstrap .card in any Portal (both TUP and Core Portal).

Note
Following code is almost identical, but allows for more specificity. Duplicated/Identical code can be found on line 174-176

/* To more specifically control c-card link font weight */
:is(.card, :--c-card) a:--c-button {
    font-weight: var(--bold);
}

Important
Matt S. mentioned that they prefer the bold text for these buttons, but could use some letter spacing. It seems to me that this is a specificity issue and seems to be overrun by

:--c-button:where(:not(:--c-button--as-link)) {

:--c-button:where(:not(:--c-button--as-link)) {
  font-weight: var(--medium);
}

Related

Changes

Importing c-card styles from tup-ui to core-styles

Testing

  • Check here Application card for button link styles
  • Check here for staff image cards (specifically h4, and last paragraph margin-bottoms)

UI

BEFORE AFTER
Screenshot 2023-10-26 at 2 20 46 PM Screenshot 2023-10-26 at 4 10 54 PM
Screenshot 2023-10-26 at 2 22 03 PM Screenshot 2023-10-26 at 3 32 14 PM
Screenshot 2023-10-26 at 2 23 19 PM Screenshot 2023-10-26 at 3 31 28 PM

@R-Tomas-Gonzalez R-Tomas-Gonzalez added feature A new feature or replacement of existing feature minor A feature in backward-compatible manner labels Oct 27, 2023
Copy link
Member

@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.

Either one or two migrated styles may be moot, thus deleted i.e. not added. Please see specific comments.

src/lib/_imports/components/c-card.css Outdated Show resolved Hide resolved
src/lib/_imports/components/c-card.css Outdated Show resolved Hide resolved
src/lib/_imports/components/c-card.css Show resolved Hide resolved
R-Tomas-Gonzalez and others added 4 commits November 2, 2023 16:41
Co-authored-by: Wesley B <62723358+wesleyboar@users.noreply.github.com>
Co-authored-by: Wesley B <62723358+wesleyboar@users.noreply.github.com>
Co-authored-by: Wesley B <62723358+wesleyboar@users.noreply.github.com>
@wesleyboar
Copy link
Member

Thank you for the notices in the description about button font-weight and letter-spacing. I've moved that to a new ticket (TUP-639), and created PRs to resolve it:

@wesleyboar wesleyboar merged commit acf9769 into main Nov 2, 2023
@wesleyboar wesleyboar deleted the feat/tup-617-c-card-fixes branch November 2, 2023 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature or replacement of existing feature minor A feature in backward-compatible manner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants