Skip to content

Conversation

@wise-king-sullyman
Copy link
Collaborator

@wise-king-sullyman wise-king-sullyman commented Jan 22, 2024

@patternfly-build
Copy link
Collaborator

patternfly-build commented Jan 22, 2024

Comment on lines 58 to 59
<div className="pf-v5-c-multiple-file-upload__status-progress-icon">{icon}</div>
<div className="pf-v5-c-multiple-file-upload__status-progress-text">{statusToggleText}</div>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed to hardcoded classes for now as I think the styling for these classes will likely be removed for #6247, which will make them unavailable on the styles object.

Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than hardcoding the whole class:

Suggested change
<div className="pf-v5-c-multiple-file-upload__status-progress-icon">{icon}</div>
<div className="pf-v5-c-multiple-file-upload__status-progress-text">{statusToggleText}</div>
<div className={`${styles.multipleFileUpload}__status-progress-icon`}>{icon}</div>
<div className={`${styles.multipleFileUpload}__status-progress-text`}>{statusToggleText}</div>

Should work

@wise-king-sullyman wise-king-sullyman requested review from a team, andrew-ronaldson, lboehling, mfrances17, nicolethoen and srambach and removed request for a team January 22, 2024 22:06
@wise-king-sullyman wise-king-sullyman linked an issue Jan 22, 2024 that may be closed by this pull request
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 comments below, otherwise lgtm

Comment on lines 58 to 59
<div className="pf-v5-c-multiple-file-upload__status-progress-icon">{icon}</div>
<div className="pf-v5-c-multiple-file-upload__status-progress-text">{statusToggleText}</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than hardcoding the whole class:

Suggested change
<div className="pf-v5-c-multiple-file-upload__status-progress-icon">{icon}</div>
<div className="pf-v5-c-multiple-file-upload__status-progress-text">{statusToggleText}</div>
<div className={`${styles.multipleFileUpload}__status-progress-icon`}>{icon}</div>
<div className={`${styles.multipleFileUpload}__status-progress-text`}>{statusToggleText}</div>

Should work

</div>
{/* TODO: Update with issue #9997. Removed class "styles.multipleFileUploadStatusItemClose" that no longer exists */}
<div>
<div className="pf-v5-c-multiple-file-upload__status-item-close">
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to above

Copy link
Collaborator

@lboehling lboehling left a comment

Choose a reason for hiding this comment

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

lgtm!

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.

css wise 👍🏼

@thatblindgeye thatblindgeye merged commit 8b89c3e into patternfly:v6 Jan 31, 2024
@wise-king-sullyman wise-king-sullyman deleted the multi-file-upload-penta-updates branch January 31, 2024 19:19
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.

Consume core Penta updates: Multi file upload

5 participants