-
Notifications
You must be signed in to change notification settings - Fork 24
Comments
fix: ensure TextLink styles are applied to nested elements#860
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not applying here other rules such as font-weight so they are preserved for nested elements
ff11f5d to
b08629c
Compare
@nats12
nats12
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good Rui! Just one comment, with Reactist updates, we tend to keep the versioning changes separate from the product updates. So in this case, we would not make changes to the package.json and lock file or the CHANGELOG, we would do it in a separate PR.
Take one of the latest changes as an example:
rmartins90
commented
Jan 17, 2025
Looks good Rui! Just one comment, with Reactist updates, we tend to keep the versioning changes separate from the product updates.
That makes sense! We should then change the PR template, as it misleads. Do you agree? (see below)
- Described changes in CHANGELOG.md
- Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
There are other examples where we change the version and CHANGELOG in the PR - d88972b
Either way, this process should be automated soon 🙂
b08629c to
1f5eb2f
Compare
rmartins90
commented
Jan 17, 2025
Removed versioning from this PR @nats12. I'm merging this now.
nats12
commented
Jan 21, 2025
That makes sense! We should then change the PR template, as it misleads. Do you agree? (see below)
There are other examples where we change the version and CHANGELOG in the PR - d88972b
Good point @rmartins90. @gnapse maybe can shed light on this: we are sometimes bumping versions in the same PR as the change, other times in separate PRs. Which process should we follow?
gnapse
commented
Jan 21, 2025
We've often bumped the version in the same PR as we make a product change (ref, ref, ref). But not always.
I don't think there's a hard rule on this. Personally, I tend to bump the version with the change, unless I have two or more changes that I know I can release together. In which case I hold off on the version bump, merge two or more PRs in a row, then do a PR with the version bump.
Regarding the PR request template, I think we can remove the "versioning" section regardless, because we already know from the PR title prefix if it's a patch release (e.g. fix:...), a minor release (e.g. feat:...) or a major release (breaking change).
rmartins90
commented
Jan 21, 2025
Meanwhile, I've created a PR to automate this process #863
Uh oh!
There was an error while loading. Please reload this page.
Short description
Hover styles (text decoration and color) were only applied to the container element itself. This caused inconsistent behavior when the text link contained nested elements, as the hover styles wouldn't propagate to the children.
This PR uses
.container:hover > *selector to target immediate children.PR Checklist
npm run validateand made sure no errors / warnings were shownCHANGELOG.mdpackage.jsonandpackage-lock.json(npm --no-git-tag-version version <major|minor|patch>) refVersioning