Make the Footer copyright line configurable - #113
Open
dcruzb wants to merge 3 commits into
Open
Conversation
The rewrite replaced v1's copyrightText prop with a hard-coded string, so every consumer of the library renders "CIn UFPE | Todos os direitos reservados" whether or not it applies to them, and there is no way to change the wording or the language. The prop comes back with that exact string as its default, so nothing changes for anyone who does not pass it. null hides the line, matching how support and contact already work in this component. This also resolves the type error in the Page story, which passed copyrightText to a Footer that no longer accepted it.
The demo string had Informatica without its accent. The point of the story is the language switch, so the English version of the same line says it more directly.
Following v1: the bottom-bar line renders only when copyrightText is passed, rather than falling back to a built-in string. A library should not assert an institution's name on behalf of whoever installs it, and a default in Portuguese naming CIn UFPE did exactly that for every consumer. Every example now passes the value explicitly, so the rendered output is unchanged everywhere it is demonstrated: both Storybook stories, the three direct Footer call sites in the Next demo, and the four that go through Page. The README already documented copyrightText as a prop. Dropping the null union with it: absent already means hidden.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The v2 rewrite replaced v1's
copyrightTextprop with a hard-coded string:Every consumer of the library renders that line, in Portuguese, naming CIn UFPE, with no way to change the wording or the language. It looks like an oversight rather than a decision:
examples/nextjs15-demo/README.mdalready documentsfooter={{ copyrightText: "CinCoders" }}, so the prop was expected to exist.What changes
copyrightText?: stringonFooterProps, with v1's semantics: the line renders when the prop is passed and is omitted when it is not. The component still supplies the©and the current year, so consumers never deal with the year rolling over.No default. A library should not assert an institution's name on behalf of whoever installs it, which is what a built-in Portuguese string naming CIn UFPE did for every consumer.
The rendered output does not change anywhere it is demonstrated
Every example passes the value explicitly:
Footer.stories.tsx— onmeta.args, so all six stories keep the line, plus aCustomCopyrightstory showing the English wordingPage.stories.tsx— the control's default, aligned to the Portuguese line the footer actually rendered<Footer … />call sites in the Next demofooter={{ … }}objects that reach Footer throughPageSide effect: it fixes the type error
Page.stories.tsxpassescopyrightTexttoFooter, complete with a declared type, a Storybook control and a default value. That has been aTS2353since the rewrite, unnoticed because v2 had no type check — Vite compiles without checking types, so the story built and rendered while the prop was silently discarded.This is the proper fix. #112 removed the story argument as a stopgap so its own CI could pass; if this merges first, that commit should be dropped from #112 so the control is not lost.
Verified locally
tsc --noEmit -p tsconfig.app.jsonclean, 14 tests passing,npm run build:libcompleting all three stages.For a reviewer to confirm
Consumers who currently rely on the line appearing without passing anything will lose it. Inside CIn that is the likely case for any app using
Pagewithfooter={{}}, so it is worth a look at whether any of them should be passing the text now.