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

fix(CodeEditor): hide button and link when read-only #9668

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

mfrances17
Copy link
Contributor

What: Closes #9537

Added code to conditionally remove the footer (Browse btn and Start from scratch link) when it's a read-only code editor. Also removed some leftover console.logs that were being sent on mounts in order to clean up the console on load.

To test, pass isReadOnly to the code editor in the Code Editor with Actions example and verify that the button and link are no longer present.

Note that there is an exception thrown on load of the code editor examples - this was pre-existing and is fixed by a separate PR: #9665

@patternfly-build
Copy link
Contributor

patternfly-build commented Sep 25, 2023

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.

This looks good. One possible reason for keeping those console logs might be to show how to get the editor value for some other use, but not a blocker to me; @nicolethoen do you recall the original intent of the console logs that are removed in this PR?

Unrelated to the linked issue, but something we may want to look into as a followup: it doesn't look like the logic for the emptyState var is working as intended. Currently on the return on line ~645 (from the current code, not in this PR), we're only rendering code that includes the emptyState if isUploadEnabled || providedEmptyState. When we declare the emptyState var on line ~519, we're assigning it either the consumer's custom providedEmptyState (from the emptyState prop), or assigning it a default based on a ternary depending on whether isUploadEnabled is true or not (if true, render the browse button + "start from scratch" button, otherwise just the "start from scratch" button).

So if we have a CodeEditor that has no code value, no emptyState prop value, and isUploadEnabled is false, the expectation seems to be that we render a default empty state with just a "Start from scratch" button (or whatever is passed to the emptyStateLink prop), like the following screenshot:

Code editor empty state without upload button

Currently what happens is the code editor just renders since the condition in that returned ternary (isUploadEnabled || providedEmptyState) is false.

Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

lgtm

@tlabaj tlabaj merged commit 0346933 into patternfly:main Oct 6, 2023
10 checks passed
@mfrances17 mfrances17 deleted the readonly-editor-rm-link branch October 24, 2023 13:50
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.

Bug - "Start from scratch" link is confusing on read-only editors with actions
4 participants