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.

Sign up for GitHub

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

Jump to bottom

feat(FontStyle): Wrong base sizes for presets 5, 9 and 10 #311

Merged
merged 1 commit into from
Oct 24, 2023
Merged

feat(FontStyle): Wrong base sizes for presets 5, 9 and 10 #311

merged 1 commit into from
Oct 24, 2023

Conversation

crisrojas
Copy link
Contributor

🎟️ Jira ticket

https://jira.tid.es/browse/SMARTWIFI-7896

🥅 What's the goal?

Conform to Mistica Design System typography specs:
https://brandfactory.telefonica.com/d/iSp7b1DkYygv/n-a#/fundamentals/typography

🚧 How do we do it?

Update baseSize property for presets 5 (20), 9 (40) and 10 (48)

@github-actions
Copy link

github-actions bot commented Oct 13, 2023 •
edited
Loading

Screenshot tests report

✔️ All passing

@crisrojas crisrojas changed the title FIX - Wrong base sizes for presets 5, 9 and 10 feat(FontStyle): Wrong base sizes for presets 5, 9 and 10 Oct 13, 2023
@amegias
Copy link
Contributor

amegias commented Oct 18, 2023

there are new screenshots which are not related to the text size changes
Are they really needed?

Captura de pantalla 2023-10-18 a las 13 21 45

@crisrojas
Copy link
Contributor Author

there are new screenshots which are not related to the text size changes Are they really needed?

Captura de pantalla 2023-10-18 a las 13 21 45

Good question 🤔. Not sure what they were regenerated. Seems there's a minor difference in the corner radius.
Would be good if we can see the output of the CI after running the tests for see if there's a problem or not about them being regenerated, but they're stuck on the SDWebImage thing...Should I still revert them ?

@amegias
Copy link
Contributor

amegias commented Oct 18, 2023

there are new screenshots which are not related to the text size changes Are they really needed?
Captura de pantalla 2023-10-18 a las 13 21 45

Good question 🤔. Not sure what they were regenerated. Seems there's a minor difference in the corner radius. Would be good if we can see the output of the CI after running the tests for see if there's a problem or not about them being regenerated, but they're stuck on the SDWebImage thing...Should I still revert them ?

I think so, I would try to revert all of screenshots except those which were failing due to font size changes

@amegias
Copy link
Contributor

amegias commented Oct 19, 2023

Tested on my laptop. 231 files should be changed (instead of 306 🤔 )

@crisrojas crisrojas force-pushed the update-presets branch 2 times, most recently from 6f7a024 to b0f7f07 Compare October 19, 2023 15:20
@crisrojas crisrojas merged commit 1a6c183 into main Oct 24, 2023
2 checks passed
@crisrojas crisrojas deleted the update-presets branch October 24, 2023 12:31
tuentisre pushed a commit that referenced this pull request Oct 25, 2023
@semantic-release-bot
chore(release): 26.3.0 [skip ci]
8858508 
# [26.3.0](v26.2.0...v26.3.0) (2023-10-25)

### Features

* **FontStyle:** Wrong base sizes for presets 5, 9 and 10 [#311](#311) ([1a6c183](1a6c183))
* **Header:** added the Small style ([281be43](281be43))
* **skin:** update design tokens from mistica-design branch: production ([#313](#313)) ([9bf2885](9bf2885))
@tuentisre
Copy link
Collaborator

🎉 This PR is included in version 26.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yceballost yceballost yceballost approved these changes

@josetorronteras josetorronteras josetorronteras approved these changes

@amegias amegias amegias approved these changes

@Gomay88 Gomay88 Gomay88 approved these changes

@alejandroruizponce alejandroruizponce alejandroruizponce approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@crisrojas @amegias @tuentisre @yceballost @josetorronteras @Gomay88 @alejandroruizponce