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: Replace link-input with link-field and social-link-field #5709

Merged
merged 28 commits into from
Nov 26, 2020
Merged

fix: Replace link-input with link-field and social-link-field #5709

merged 28 commits into from
Nov 26, 2020

Conversation

divyamtayal
Copy link
Member

@divyamtayal divyamtayal commented Nov 21, 2020

Fixes #5570

Short description of what this resolves:

Replace all link-input component calls with link-field and social-link-field and then remove the component and all segmented link shenanigans from the project

TODO

  • Replace LinkInput with LinkField or SocialLinkField
  • Remove Segmented Links
  • Remove Transation for LinkInput
  • Take care of styling

Checklist

  • I have read the Contribution & Best practices Guide.
  • My branch is up-to-date with the Upstream development branch.
  • The acceptance, integration, unit tests and linter pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)

@vercel
Copy link

vercel bot commented Nov 21, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/eventyay/open-event-frontend/8xu4kz86p
✅ Preview: https://open-event-frontend-git-link-refactor-5570.eventyay.vercel.app

@codecov
Copy link

codecov bot commented Nov 21, 2020

Codecov Report

Merging #5709 (16bb513) into development (36c936e) will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #5709      +/-   ##
===============================================
- Coverage        23.69%   23.60%   -0.09%     
===============================================
  Files              510      509       -1     
  Lines             5415     5375      -40     
  Branches            59       59              
===============================================
- Hits              1283     1269      -14     
+ Misses            4116     4090      -26     
  Partials            16       16              
Impacted Files Coverage Δ
app/controllers/events/list.js 23.07% <ø> (ø)
app/models/custom-form.js 30.76% <ø> (+2.19%) ⬆️
app/models/event.js 50.00% <ø> (ø)
app/models/session.js 10.00% <ø> (ø)
app/models/social-link.js 0.00% <ø> (ø)
app/models/speaker.js 0.00% <ø> (ø)
app/utils/computed-helpers.js 24.13% <ø> (+4.57%) ⬆️
app/components/tabbed-navigation.js 33.33% <0.00%> (-20.00%) ⬇️
app/components/public/side-menu.js 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36c936e...16bb513. Read the comment docs.

@divyamtayal
Copy link
Member Author

@iamareebjamal Please let me know that should I remove segmented links where they where never being used also?
I have refactored almost everything.
Please see the changes done.

@iamareebjamal
Copy link
Member

Yes, remove all segmented links and also the helper code

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 22, 2020

@iamareebjamal should I not delete linkinput comp files?? Also its test file??

@iamareebjamal
Copy link
Member

Commented by mistake. Thought its link-field test

@divyamtayal
Copy link
Member Author

divyamtayal commented Nov 22, 2020

Any other changes to make?

@iamareebjamal
Copy link
Member

Will test and merge tomorrow

@iamareebjamal
Copy link
Member

@daretobedifferent18 Please check your gitter

@divyamtayal
Copy link
Member Author

@daretobedifferent18 Please check your gitter

ok

@iamareebjamal
Copy link
Member

@maze-runnar Please review and test locally that everything is working as expected

@maze-runnar
Copy link
Contributor

@maze-runnar Please review and test locally that everything is working as expected

ok

@maze-runnar
Copy link
Contributor

it's working fine, but ui field in mobile view -
scrnli_11_24_2020_3-59-25 PM
scrnli_11_24_2020_4-00-48 PM

@divyamtayal
Copy link
Member Author

Screenshot from 2020-11-24 20-40-11

@divyamtayal
Copy link
Member Author

@iamareebjamal I have made all the changes asked for.

@iamareebjamal iamareebjamal changed the title Replace link-input with link-field and social-link-field fix: Replace link-input with link-field and social-link-field Nov 26, 2020
@auto-label auto-label bot added the fix label Nov 26, 2020
@iamareebjamal iamareebjamal merged commit 5db1c7a into fossasia:development Nov 26, 2020
@divyamtayal divyamtayal deleted the link-refactor-5570 branch November 26, 2020 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace link-input with link-field and social-link-field
3 participants