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

Add description list basic demo #10184

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

ajaypratap003
Copy link
Member

What: Closes #7000
Add description list basic demo

Additional issues:

@patternfly-build
Copy link
Contributor

patternfly-build commented Mar 21, 2024

.gitignore Outdated Show resolved Hide resolved
@tlabaj tlabaj requested review from a team, mattnolting and mmenestr and removed request for a team April 17, 2024 13:51
Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

It looks like the content here is pretty different from the content of the core demo, is that something we're concerned with on these @tlabaj ?

Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

Couple more nits below but otherwise looking good to me

@tlabaj
Copy link
Contributor

tlabaj commented Apr 26, 2024

It looks like the content here is pretty different from the content of the core demo, is that something we're concerned with on these @tlabaj ?

I agree with Austin, the content should match core as closely as possible. Could we update the data in the card to more closely align wit core please.

Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

It looks like there's still some items that aren't lining up with the core demo, for instance the divider not going all the way across the card, the distance between the divider and the heading, the layout and order of the items, and the status being a link:

image

@kmcfaul
Copy link
Contributor

kmcfaul commented May 8, 2024

Still seeing the divider mismatch on the latest surge.

@ajaypratap003
Copy link
Member Author

Still seeing the divider mismatch on the latest surge.

@kmcfaul @wise-king-sullyman On local changes look fine but seems changes are not reflecting on surge.

image

Copy link
Contributor

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

Yep I see that same thing, not sure why

@ajaypratap003
Copy link
Member Author

Still seeing the divider mismatch on the latest surge.

Seems changes are not reflecting on surge. Please check it on local. It seems fine on local.

Copy link
Contributor

@mattnolting mattnolting left a comment

Choose a reason for hiding this comment

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

Code looks great! We need to get the previews working to confirm.

@nicolethoen
Copy link
Contributor

@mattnolting I think the preview is fixed if you wouldn't mind a quick CSS review

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

Successfully merging this pull request may close these issues.

Description list basic demo
8 participants