-
Notifications
You must be signed in to change notification settings - Fork 1
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: Apps-2922 remove time stamp on homepage #637
Merged
Merged
Conversation
This file contains 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
Percy ScreenshotsIn order to conserve our percy screenshot allowance, percy is not configured to run automatically. Please make sure the PR is ready and all other checks are passing, then start it manually:
|
github-actions
bot
temporarily deployed
to
storybook--pull_request-637
October 18, 2024 00:12
Inactive
github-actions
bot
temporarily deployed
to
storybook--pull_request-637
October 18, 2024 00:18
Inactive
github-actions
bot
temporarily deployed
to
storybook--pull_request-637
October 18, 2024 03:38
Inactive
…hub.com/UCLALibrary/ucla-library-website-components into APPS-2922_remove_time_stamp_on_homepage
github-actions
bot
temporarily deployed
to
storybook--pull_request-637
October 18, 2024 16:44
Inactive
4 tasks
github-actions
bot
temporarily deployed
to
storybook--pull_request-637
October 18, 2024 18:04
Inactive
github-actions
bot
temporarily deployed
to
storybook--pull_request-637
October 18, 2024 18:13
Inactive
github-actions
bot
temporarily deployed
to
storybook--pull_request-637
October 18, 2024 21:03
Inactive
tinuola
approved these changes
Oct 18, 2024
pghorpade
pushed a commit
that referenced
this pull request
Oct 18, 2024
## [3.29.5](v3.29.4...v3.29.5) (2024-10-18) ### Bug Fixes * Apps-2922 remove time stamp on homepage ([#637](#637)) ([f1c6e49](f1c6e49))
🎉 This PR is included in version 3.29.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
pghorpade
pushed a commit
that referenced
this pull request
Oct 22, 2024
* test: trying to recreate * fix: time on ucla homepage * chore: cleanup debugging mess, add test * fix: prevent datecreated in default * fix: undefined theme case * fix: computed method for theme logic * fix: closing tag --------- Co-authored-by: Jess Divers <[email protected]> Co-authored-by: JenDiamond <[email protected]>
pghorpade
pushed a commit
that referenced
this pull request
Oct 22, 2024
## [3.29.5](v3.29.4...v3.29.5) (2024-10-18) ### Bug Fixes * Apps-2922 remove time stamp on homepage ([#637](#637)) ([f1c6e49](f1c6e49))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Connected to APPS-2922
Component Updated: CardMeta.vue
Stories: ~/stories/SectionWrapper.stories.js
Notes:
The main issue was the theme !== undefined check, which was meant to return false on the
library-website-nuxt
site. However, during testing I realized the site has a theme now, set to''
. So the undefined check was being passed and the time was rendering when it wasn't meant to. To fix this I made cardmeta explicitly check for ftva theme to show the time.While testing this locally in library-website-nuxt, I noticed that we now had 2 dates showing (screenshot below). This is because of the postDate changes we made 15ish days ago that had not yet been updated on the library-website-nuxt site. To prevent this, I added a check to BlockCardWithImage to only pass the postDate when we are using the
ftva
theme.Finally I added a story to SectionWrapper with the same data and theme setup as the library site homepage so this bug can be checked for.
Lastly, Our previous default stories without theme injections should maybe be updated everywhere - as they are no longer accurate. Should I do that as part of this ticket (add 'theme = '' to all default stories)?
Checklist:
library-website-nuxt dev server
[ ] UX has reviewed and approved this