Conversation
Rucha1499
requested review from
SagarBajpai,
ankushdharkar,
sumitd94,
swarajpure and
whyDontI
swarajpure
left a comment
swarajpure
left a comment
There was a problem hiding this comment.
Congrats for starting to learn ember and on your first PR in ember 🥳
Requesting changes:
Need to include Ember Test Selectors
Need to remove the hardcoding in template
Also some CSS related minor stuff
Thank you! 😄
app/styles/self-clear-cache.css
Outdated
| color:#041187; | ||
| font-family: Helvetica Neue; | ||
| font-weight: bold; | ||
| font-size: 36px |
There was a problem hiding this comment.
Please refer this review comment for font-size: #78 (comment)
app/styles/self-clear-cache.css
Outdated
|
|
||
|
|
||
| .button { | ||
| top: 335px; |
There was a problem hiding this comment.
Can we use something like % here?
app/styles/self-clear-cache.css
Outdated
| background-color: white; | ||
| font-family: Helvetica Neue; | ||
| font-weight:bold; | ||
| font-size: 18px; |
app/styles/self-clear-cache.css
Outdated
| background-color: transparent; | ||
| border: 2px solid #FF8C00; | ||
| color: #FF8C00; | ||
| background-color: white; |
There was a problem hiding this comment.
background-color is repeating
There was a problem hiding this comment.
Thanks for pointing it out! 😃
app/styles/self-clear-cache.css
Outdated
| color:#041187; | ||
| font-family: Helvetica Neue; | ||
| font-weight: bold; | ||
| font-size: 24px |
app/templates/index.hbs
Outdated
| @@ -1 +1 @@ | |||
| Index page works! No newline at end of file | |||
| <SelfClearCache @time="23 March 1:23 pm IST" @totalTimes="1"/> No newline at end of file | |||
There was a problem hiding this comment.
Once we have the API ready:
- We would be calling that API from
routesfile - Pass that response in
model - Get the data from
modelin controller (for this template) - Mark it as
@tracked(state variable) - Then finally use that tracked variable here (in template) as props.
If this sounds confusing, please read that again and refer this PR to get an idea of what I'm trying to say: https://github.com/Real-Dev-Squad/website-my/pull/82/files
I know we don't have the API yet, so we could create a mock object in controller file and get that mock data from controller to template, so we can remove the hardcoding and our way to integrating API (when it's ready) will become easier, less work to do then.
Let me know if there are any doubts 🙂
There was a problem hiding this comment.
Thanks for the review and the explanation! It will surely help a lot. 💯
app/styles/self-clear-cache.css
Outdated
|
|
||
| } |
There was a problem hiding this comment.
Unnecessary empty line before every closing bracket
app/components/self-clear-cache.hbs
Outdated
| <div class="self-clear-cache-container"> | ||
| <div class="last-request"> | ||
| Last Request: {{@time}} | ||
| </div> | ||
| <div class="clear-cache-button"> | ||
| <button class="button" type="submit" disableb={{false}}>Clear cache</button> | ||
| </div> | ||
| <div class="remaining-requests"> | ||
| {{@totalTimes}} / 3 requests remaining for today | ||
| </div> | ||
| </div> No newline at end of file |
There was a problem hiding this comment.
Okay! I will make the changes.
ankushdharkar
added
feature task
app/styles/self-clear-cache.css
Outdated
| } | ||
|
|
||
| .button { | ||
| top: 335%; |
app/components/self-clear-cache.hbs
Outdated
| Last Request: {{@time}} | ||
| </div> | ||
| <div class="clear-cache-button"> | ||
| <button class="button" type="submit" disableb={{false}}>Clear cache</button> |
app/components/self-clear-cache.hbs
Outdated
| @@ -0,0 +1,11 @@ | |||
| <div class="self-clear-cache-container"> | |||
There was a problem hiding this comment.
NIT: Can we please follow BEM rule for class naming?
…into feat/UI-cache-page
5 participants
Create UI for clearing cache on index page. Fixes #66